-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
default argparser #916
Comments
Something like this as a static method of Trainer or as a util works? |
@skepticleo looks good, could you send a PR? 🤖 |
This would go really nicely with a classmethod/overload that constructs the Trainer from the passed arguments.
Then the user's pattern becomes:
|
In the arguments some of the default values are set to None to comply with the the Trainer's constructor. In my case I often pass and save my args into the LightningModule as self.hparams. When there are values of None associated with an argument in the Namespace object Tensorboard will raise a ValueError exception as it is not an int, float, str, bool or torch.tensor By modifying the defaults to be acceptable alternatives (ie. for --gpus, default=0) this issue can be avoided (Assuming a work around exists currently for each Trainer parameter) |
@Borda Honestly I wasn't sure which way would be the best to address it, locally I have just added a dictionary to catch the cases that cause the issue, but that is hard to maintain Ultimately it would be better to adjust the default arguments in the constructor from None to some other value, but I don't have enough scope to fully understand the implications of that for legacy support or constructor code I also had a pending pull request on @skepticleo 's fork that parsed the constructor doc string to extract one line help messages for the arguments. Should I look to include this as well? |
You can convert params to dictionary (temporary) and filter items to be not None... |
Sure, but then the user can't use some of the arguments like And then in some cases there are explicit checks for a NoneType in the constructor like for So you end up in a position of filtering them before saving as hparams, but after instancing the Trainer (which causes you to lose some information about your settings) or not including them in the default argparser removing a large amount of common use from it, multi gpu settings, save paths, ect I can't see a good bandaid here, it requires the removal of NoneTypes as defaults from the constructors signature and a repair of what that impacts |
🚀 Feature
Create a default argparser with all the properties that can go into a trainer
Motivation
People already do this themselves pretty often. Might as well make it easy for them
The text was updated successfully, but these errors were encountered: