[feat] Add flexible step-wise LR scheduler with minimum changes #256
Conversation
Codecov Report
@@ Coverage Diff @@
## development #256 +/- ##
===============================================
- Coverage 81.62% 81.62% -0.01%
===============================================
Files 150 151 +1
Lines 8625 8646 +21
Branches 1325 1328 +3
===============================================
+ Hits 7040 7057 +17
- Misses 1108 1111 +3
- Partials 477 478 +1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Hey, Thanks for this PR. The changes look good. However, I still don't see where epoch or batch is passed by an LR scheduler. So basically, how does the fit dictionary 'X' get updated to contain the step unit? To that end, could we add a test where we run a few schedulers in a training and we verify that the step was made at the appropriate time. This can be achieved by using the internal parameters of the torch scheduler for example, in CosineAnnealingWarmRestarts we have T_0 or T_curr which contains the epochs since the last step. And we can verify if it is done epoch wise or batch wise there.
I will check promptly and give another commit soon
Should I do it separetly as in the latest commit of test codes by me or should I really run it from API? |
yeah so the idea is to train a pipeline with the different schedulers and make sure that the step is made at the right time. The code you added in itself might run, but the tests I would like to see would ensure that proper information is passed by the chosen lr scheduler and it is able to take a step at the appropriate time. This will ensure that CosineAnnealing or some other scheduler if selected in a configuration is able to take the appropriate step |
Since we would like to merge this feature promptly, I cut this new branch from the branch hot-fix-adapt... and narrowed down the scope of this PR. The subsequent PR addresses the issues, epspecially the format and mypy typing.
The intention behind the change from torch.tensor to torch.Tensor is that since I got an error NoModuleFound `import torch.tensor`. Plus, the torch.tensor is not a TensorType class, but torch.Tensor is. Therefore, I changed the torch.tensor to torch.Tensor.
Since the previous version always use batch-wise update, I added the step_unit = batch and then avoid the errors I got from pytest.
Since the latest version only supports the batch-wise update, I just inserted step_unit == "batch" to be able to run greedy portfolio selection.
…ons in base_scheduler
nabenabe0928
force-pushed
the
feat-epoch-wise-LR-scheduler
branch
from
ae354c8
to
8494e0c
Compare
Since the step_interval should not be modified from outside, I made it a property of the base_scheduler class. Furthermore, since we do not have to check the type of step_interval except the initialization, I deleted the type check from prepare method.
autoPyTorch/pipeline/components/training/trainer/base_trainer.py
Outdated
Show resolved
Hide resolved
Since this PR is urgent now, I made another PR to quickly merge the feature.
The original PR is available here.