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

[onnx] Decompose onnx.Adam into elementwise operations #3643

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

Conversation

rsuderman
Copy link
Contributor

We can support onnx.Adam by decomposing into elementwise operations. The numerics slightly differ from onnxruntime however they appear to be within margins.

We can support `onnx.Adam` by decomposing into elementwise operations.
The numerics slightly differ from `onnxruntime` however they appear to
be within margins.
Comment on lines +156 to +160
float alpha = 0.9;
float beta = 0.999;
float epsilon = 0.0;
float norm = 0.0;
float postNorm = 0.0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a need to initialize all of them since the default value would be assigned during the binding itself? Also, can we declare them all in the same line?

Comment on lines +172 to +175
SmallVector<Value> inputs;
SmallVector<Value> grads;
SmallVector<Value> accGrads;
SmallVector<Value> accSqGrads;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here for declaration.

Comment on lines +242 to +251
SmallVector<Value> newX;
SmallVector<Value> newG;
SmallVector<Value> newSG;

for (int i = 0, s = inputs.size(); i < s; ++i) {

Value x = inputs[i];
Value g = grads[i];
Value v = accGrads[i];
Value h = accSqGrads[i];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here as above.

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