Argument Resolution Refactoring #184
Conversation
…nt_resolution # Conflicts: # delira/training/base_experiment.py
There was a problem hiding this comment.
I reviewed the pytorch experiment and the baseexperiment (most of the comments should apply to the other experiments, too). Really digging the new argument structure, much more intuitive. Added a bit more comment than usual because this is a good opportunity to improve the general interface of delira.
Further considerations include:
-
Where is the batchsize determined. After some thought, I really liked the idea of you to use the batchsize from the datamanager.
-
Where is a good point to mention which arguments need to be passed by the parameters object | datamanager | experiment ? (the experiment args are obvious but the parameters and datamangers aren't)
-
The notebooks should be updated accordingly, too. (This should probably happen after Fix notebooks and trainer indent #182)
-
Do we want to include typings for the experiment interfaces? (I think this would be the right to do that)
|
All the changes you requested are making sense to me, so I'll add them later on. Regarding your considerations:
|
mibaumgartner
added
in progress
Fixes #180 once it's done.
Tests have not yet been updated. Before doing this, a feedback on the general structure would be nice