Skip to content

Conversation

@DNXie
Copy link
Member

@DNXie DNXie commented Oct 28, 2025

Now the lr starts from 1e-05 (specified in the config) instead of 0.001

Log:

rl_trainer/learning_rate: 1e-05
image

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Oct 28, 2025
@DNXie DNXie requested a review from felipemello1 October 28, 2025 22:29
@DNXie
Copy link
Member Author

DNXie commented Oct 28, 2025

cc @wukaixingxp

@DNXie DNXie requested a review from casteryh October 28, 2025 22:29
@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.60%. Comparing base (248126a) to head (b3603ef).
⚠️ Report is 44 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #507       +/-   ##
===========================================
+ Coverage   63.57%   84.60%   +21.03%     
===========================================
  Files          77       30       -47     
  Lines        7569     3807     -3762     
===========================================
- Hits         4812     3221     -1591     
+ Misses       2757      586     -2171     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@wukaixingxp
Copy link
Contributor

wukaixingxp commented Oct 28, 2025

Thanks for the quick fix, just curious what should we do if self.engine.lr_schedulers.schedulers has more than one lr_scheduler in the list.. any reason we make the self.engine,lr_schedulers.schedulers a list?

Copy link
Contributor

@casteryh casteryh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm.
Would it make sense to record all the lrs of each scheduler? (I have no idea why there are multiple schedulers in the first place tbh).

@DNXie
Copy link
Member Author

DNXie commented Oct 29, 2025

@casteryh @wukaixingxp
I am not 100% sure why there would be multiple lr schedulers either..
But according to the docstring here, all the lr schedulers are the same. And in this example, it also only accesses the first lr_scheduler.

@DNXie DNXie merged commit d4f0f78 into meta-pytorch:main Oct 29, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants