Skip to content

Updated mu_product initialization - #1155

Merged
NiklasGustafsson merged 3 commits into
dotnet:mainfrom
shaltielshmid:fix-nadam-muproduct
Nov 27, 2023
Merged

NiklasGustafsson merged 3 commits into
dotnet:mainfrom
shaltielshmid:fix-nadam-muproduct

Conversation

@shaltielshmid

Copy link
Copy Markdown
Contributor

Following the issue here, the mu_product is now initialized to 1.

@shaltielshmid

Copy link
Copy Markdown
Contributor Author

In addition to the bug with mu_product never being initialized, I discovered that mu_product_next wasn't being calculated like in PyTorch.
In TorchSharp: var mu_product_next = mu_product * mu * mu_next; See here
In PyTorch: mu_product_next = mu_product * mu_next See here

Those two changes resulted in the tests failing due to the optimization calculation being changed. I updated the scores, and confirmed that they match the scores from PyTorch. Attaching code to run the NAdam tests in Python here.

@NiklasGustafsson
NiklasGustafsson merged commit f31c8d3 into dotnet:main Nov 27, 2023
@shaltielshmid
shaltielshmid deleted the fix-nadam-muproduct branch November 27, 2023 18:09
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