Skip to content

Introduce VariadicAlias, remove hardcoded alias limits#6106

Merged
SherlockNoMad merged 4 commits intomasterfrom
bahuang/alias
Dec 11, 2020
Merged

Introduce VariadicAlias, remove hardcoded alias limits#6106
SherlockNoMad merged 4 commits intomasterfrom
bahuang/alias

Conversation

@SherlockNoMad
Copy link
Copy Markdown
Contributor

Description: Introduce VariadicAlias, remove hardcoded alias range limits

@SherlockNoMad SherlockNoMad added the training issues related to ONNX Runtime training; typically submitted using template label Dec 10, 2020
@SherlockNoMad SherlockNoMad requested a review from wschin December 10, 2020 23:55
@SherlockNoMad SherlockNoMad requested a review from a team as a code owner December 10, 2020 23:55
}

const nonstd::optional<std::pair<int, int>>& VariadicAlias() const {
return variadic_alias_offsets_;
Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
Contributor Author

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.

Copy link
Copy Markdown
Contributor

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.

wschin
wschin previously approved these changes Dec 11, 2020
Copy link
Copy Markdown
Contributor

@wschin wschin left a 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.

@SherlockNoMad SherlockNoMad merged commit a53f4dd into master Dec 11, 2020
@SherlockNoMad SherlockNoMad deleted the bahuang/alias branch December 11, 2020 18:47
pengwa pushed a commit that referenced this pull request Dec 28, 2020
* 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

training issues related to ONNX Runtime training; typically submitted using template

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants