Remove Automatic Memory Optimization Attribute From Model #176
Conversation
| @@ -719,7 +718,6 @@ class TestNTNHighMemory(_BaseNTNTest): | |||
|
|
|||
| model_kwargs = { | |||
| 'num_slices': 2, | |||
| 'automatic_memory_optimization': False, | |||
| } | |||
There was a problem hiding this comment.
Now, TestNTNLowMemory = TestNTNHighMemory?
There was a problem hiding this comment.
don't forget to explicitly set the evaluator's automatic_memory_optimization
There was a problem hiding this comment.
Now i'm wondering if/how these should get passed through to the CLI, which tests the whole pipeline
There was a problem hiding this comment.
I solved this in b973e7c, but now I'm wondering if it makes sense to store this as a boolean in instances of the Evaluator class, or rather to just make it an argument to the Evaluator.evaluate() function. This is the last thing I'd like to discuss before finishing/merging this PR (besides passing CI ;))
There was a problem hiding this comment.
I was also thinking about it, and in the initial commits, we defined amo as a function argument of Evaluator.evaluate(), and TrainingLoop.train(). But when defining the EarlyStopper, we need to pass the information to the Evaluator, too, so that we might need to define amo as an attribute of the EarlyStopper. Therefore, I prefer to define it directly as an attribute of the Evaluator/TrainingLoop
There was a problem hiding this comment.
I think it is something which you either want to have for the whole training / evaluation or not; so it won't change for multiple calls of the evaluate method, which is why storing it as state of the evaluator seems fine for me.
There was a problem hiding this comment.
Okay then everything is good as it stands
There was a problem hiding this comment.
Excellent idea, @mali-git. Please see my comments - most are related to making sure that both the training loop and evaluator's automatic memory optimization is set explicitly now that it doesn't inherit from the model, as well as the usage of subclasses' super().__init__() to forward variables rather than setting in the subclass
Automatic memory optimization is performed by the TrainingLoop/Evaluator. Therefore, in this PR, we define the automatic memory optimization flag as an attribute of these objects.