chengtan9907 / OpenSTL

OpenSTL: A Comprehensive Benchmark of Spatio-Temporal Predictive Learning
https://openstl.readthedocs.io/en/latest/
Apache License 2.0
776 stars 129 forks source link

Fix a bug for onecycle scheduler usage in Pytorch Lightening Version #113

Closed yyyujintang closed 8 months ago

yyyujintang commented 8 months ago

I've observed a performance degradation when employing the onecycle scheduler, which appears to diverge from the results presented in your report. Upon investigation, I identified the source of the issue to be the lr_scheduler_step function defined within openstl/methods/base_method.py. The current implementation is as follows:

def lr_scheduler_step(self, scheduler, *args, **kwargs):
        scheduler.step(epoch=self.current_epoch)

However, the onecycle scheduler operates on a per-iteration basis rather than per-epoch. Therefore, I suggest modifying the function to simply invoke scheduler.step() without passing the epoch, as shown below:

def lr_scheduler_step(self, scheduler, *args, **kwargs):
       scheduler.step()

This adjustment should align the scheduler's step function with the expected behavior for onecycle scheduler, potentially resolving the observed performance degradation.

chengtan9907 commented 8 months ago

Thank you a lot for your valuable suggestion! There is a conflict between lr_scheduler_step and configure_optimizers. I think removing the lr_scheduler_step will help.

chengtan9907 commented 8 months ago

I have fixed this bug in the commit a6e0dd3.