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

Passing Trainer/Tuner method's (fit, predict) arguments through YAML or CLI #99

Closed
ibro45 opened this issue Jan 29, 2024 · 3 comments
Closed
Labels
enhancement New feature or request

Comments

@ibro45
Copy link
Collaborator

ibro45 commented Jan 29, 2024

If we allow only a certain root level config entries such as trainer, system, and whatever we end up calling CONSTANTS, we could also reserve the following: fit, predict, validate, test, tune, lr_find, and scale_batch_size (the last two are from Tuner). These would be passed directly to their respective argument when run.

Then either through CLI or YAML we could specify for example

lighter fit --config_file continue.yaml --fit#ckpt_path="./trained.ckpt"

fit#ckpt_path: trained.ckpt
# or
fit:
    ckpt_path: trained.ckpt
@ibro45 ibro45 added the enhancement New feature or request label Jan 29, 2024
@ibro45 ibro45 changed the title Passing Trainer/Tuner method's (fit, predict, tune) arguments through YAML or CLI Passing Trainer/Tuner method's (fit, predict) arguments through YAML or CLI Jan 29, 2024
@project-lighter project-lighter deleted a comment from dosubot bot Jan 30, 2024
@surajpaib
Copy link
Collaborator

I think this is still not an ideal solution. Not that I can think of one.

I would prefer having something like method_args instead of each method actually reserved and defined. This way ckpt_path can be used for both fit, validate, test and predict if necessary.

If we want to keep each isolated then we should also somehow enforce arguments that can be provided there, which we wouldn't want to do.

@ibro45
Copy link
Collaborator Author

ibro45 commented Mar 25, 2024

Any recent thoughts on this?

This prevents us from continuing training from a PL checkpoint or using the Tuner easily.

@ibro45
Copy link
Collaborator Author

ibro45 commented May 17, 2024

This way ckpt_path can be used for both fit, validate, test and predict if necessary.

I would avoid this, gets confusing. I can easily think of it happening with continuing the training with fit and ckpt_path, and then wanting to predict, but forgetting to update method_args#ckpt_path. If the same value is needed, I'd rather explicitly refer to it from the other one, %fit#ckpt_path.

Also, a lot of these methods won't share the same arguments so there's more problems with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants