-
Notifications
You must be signed in to change notification settings - Fork 49
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
Bug fix: Provide default for MLLayerNormalizationOptions axes in steps #538
Bug fix: Provide default for MLLayerNormalizationOptions axes in steps #538
Conversation
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.
LGTM, thanks for the fix!
917c369
to
77e0977
Compare
This adds an explicit algorithm step in layerNormalization() to provide a default described only in prose for the axes option. For webmachinelearning#211
77e0977
to
3a9acd5
Compare
@fdwr can you review (and merge if it looks good?) - thanks! |
Editing wording for 0D and 1D case.
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.
LGTM!
Oh, we still ought to update the prose too, as it still says: "When this member is not present, it is assumed to be [1,2,3] that is..." I'll push another iteration and await re-review from Ningxin or Josh. |
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.
LGTM
(The build error is due to the lingering {{MLCommamdEncoder}} reference) |
This adds an explicit algorithm step in layerNormalization() to provide a default described only in prose for the axes option.
For #211
Preview | Diff