Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

simplification over all kinds of "path" settings #882

Closed
marsggbo opened this issue Feb 17, 2020 · 7 comments
Closed

simplification over all kinds of "path" settings #882

marsggbo opened this issue Feb 17, 2020 · 7 comments
Labels
docs Documentation related feature Is an improvement or enhancement good first issue Good for newcomers won't fix This will not be worked on

Comments

@marsggbo
Copy link
Contributor

marsggbo commented Feb 17, 2020

I'm lost in all kinds of "path" settings, could it be more simpler?

Across the source code and examples, I find that there are many types of "path" config, such as the path in ModelCheckpoint, logger, and default_save_path in trainer.

Could you please explain these path configs in more detail? For example, if we set default_save_path, how does it influence other path configs?

@marsggbo marsggbo added docs Documentation related typo labels Feb 17, 2020
@github-actions
Copy link
Contributor

Hey, thanks for your contribution! Great first issue!

@Borda Borda removed the typo label Feb 19, 2020
@hadim
Copy link
Contributor

hadim commented Feb 19, 2020

I agree this kind be confusing. I am trying to unify the callback system and also try to merge this with the loggers.

In that spirit, I have added a property in Callback to retrieve what I would call the "training base directory": https://github.com/PyTorchLightning/pytorch-lightning/pull/889/files#diff-c2e9d63e89e5e7cc3384083af905407dR29

By default, all callbacks should use this path and only use the one provided in the init of the callback if it's different than None.

Callback that write on disk should also have a prefix arguments so all the files they write would go in base_training_folder / prefix (it can be a folder or a file).

Let me know what do you think.

@Borda
Copy link
Member

Borda commented Feb 21, 2020

@marsggbo good suggestion, we shall keep the API simple and easy to navigate...
Would you consider sending a PR with such simplification so we may have a concrete discussion over each case?

@Borda Borda changed the title I'm lost in all kinds of "path" settings, could it be more simpler? simplification over all kinds of "path" settings Feb 21, 2020
@Borda Borda added feature Is an improvement or enhancement good first issue Good for newcomers labels Feb 21, 2020
@marsggbo
Copy link
Contributor Author

@Borda Hello, thank you for this great project!

Actually, I'm not familiar with the code details. However, I implement a framework, namely torchline, based on pytorch-lightning. In torchline, all configuration can be set in a single yaml file by using yacs lib. I think that it may further simplify the code quantity and improve the code reusability.

To keep the path consistency, we do some preprocessing when initializing the config, refering to torchline/config/config.py.

hope it can help you.

@Borda
Copy link
Member

Borda commented Feb 24, 2020

Good suggestion, could you pls participate in #807?
Could we continue here with "path" so @hadim do you that #849 and #889 solves this completely?

@stale
Copy link

stale bot commented Apr 24, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the won't fix This will not be worked on label Apr 24, 2020
@Borda Borda removed the won't fix This will not be worked on label Apr 24, 2020
@stale
Copy link

stale bot commented Jun 23, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation related feature Is an improvement or enhancement good first issue Good for newcomers won't fix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants