-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Introduce VariadicAlias, remove hardcoded alias limits #6106
Conversation
@@ -64,6 +65,10 @@ class KernelDef { | |||
return alias_map_; | |||
} | |||
|
|||
const nonstd::optional<std::pair<int, int>>& VariadicAlias() const { | |||
return variadic_alias_offsets_; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
variadic_alias_offsets_ [](start = 11, length = 23)
How would it be used to represent Lamb's schema such as [eta, w0, g0, m0, v0, w1, g1, m1, v1] -> [w0, m0, v0, w1, g1, v1]? I think we need an initial bias and a vector to specify the input-to-output mapping in a peridic pattern. For my example here, the vector is [0, -1, 1, 2] where '-1' mean 'g0' is not mapped to output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lamb is too special a case, I will leave it out, to make the interface clean and simple.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many optimizers may fall into that category, not only Lamb.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR removes the input-output-length limit on several operators, which is good. I hope we can have a long-term solution for all operators with variadic-input/output list, but if this unblocks a production model, I am fine to merge this PR.
* Introduce VariadicAlias, remove hardcoded alias limits * Include optional-lite in winml build Co-authored-by: Sherlock Huang <bahuang@OrtTrainingDev3.af05slrtruoetgaxwwjv5nsq5e.px.internal.cloudapp.net> (cherry picked from commit a53f4dd)
Description: Introduce VariadicAlias, remove hardcoded alias range limits