Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary learning_rate arguments #5415

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

vincentpierre
Copy link
Contributor

Proposed change(s)

Removing the learning rate argument from some of our optimizers. These optimizers update their learning rate before each update using the scheduler. Passing the learning rate as argument is confusing because this argument does not actually do anything (I got tricked once).

Useful links (Github issues, JIRA tickets, ML-Agents forum threads etc.)

Types of change(s)

  • Bug fix
  • New feature
  • Code refactor
  • Breaking change
  • Documentation update
  • Other (please describe)

Checklist

  • Added tests that prove my fix is effective or that my feature works
  • Updated the changelog (if applicable)
  • Updated the documentation (if applicable)
  • Updated the migration guide (if applicable)

Other comments

@vincentpierre vincentpierre self-assigned this Jun 3, 2021
@vincentpierre vincentpierre changed the title Develop remove unnecessary lr Remove unnecessary learning_rate arguments Jun 3, 2021
@miguelalonsojr miguelalonsojr requested review from miguelalonsojr and removed request for ervteng and andrewcoh January 18, 2022 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants