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

CWR* and AR1 optimizer initialization #79

Closed
AntonioCarta opened this issue Jun 12, 2020 · 2 comments
Closed

CWR* and AR1 optimizer initialization #79

AntonioCarta opened this issue Jun 12, 2020 · 2 comments
Labels
bug Something isn't working

Comments

@AntonioCarta
Copy link
Collaborator

CWR* and AR1 can either use an optimizer passed by the caller, or create one with the lr and momentum parameters.

I think the optimizer should not be an argument because right now they are silently overriding the user's choice. Let me know if there is any reason for this otherwise I will remove the parameter.

@vlomonaco vlomonaco assigned vlomonaco and unassigned vlomonaco Jun 16, 2020
@vlomonaco vlomonaco added the bug Something isn't working label Jun 16, 2020
@vlomonaco
Copy link
Member

that's a bug. The user should be able to change them!

@AntonioCarta
Copy link
Collaborator Author

Actually, I was wrong. I checked again and everything is working as expected. Sorry for the mistake, I was probably confused by the double call to the parent constructor.
CWR* and AR1 have a slightly different interface compared to other strategies w.r.t. optimizer initialization but there is nothing wrong with it.

I'm closing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants