-
Notifications
You must be signed in to change notification settings - Fork 6
[Fusilli] Implemented Layernorm node #46
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
base: main
Are you sure you want to change the base?
Conversation
d7b8357 to
9979c82
Compare
77f22f2 to
e92b893
Compare
Signed-off-by: Alexandra Sidorova <[email protected]>
e92b893 to
8f2f63e
Compare
|
@IanWood1 @sjain-stanford hi! Could you please start review? |
| layernorm(const std::shared_ptr<TensorAttr> &x, | ||
| const std::shared_ptr<TensorAttr> &scale, | ||
| const std::shared_ptr<TensorAttr> &bias, | ||
| const std::shared_ptr<TensorAttr> &epsilon, |
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.
The signature for cudnn layernorm only uses 3 tensor inputs: x, scale and bias. Why do we need epsilon to be a Tensor input here?
| // Layer normalization node. | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| class LayernormNode : public NodeCRTP<LayernormNode> { |
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.
We also need Layernorm backward at some point (follow-on is fine). It's called DLNNode in cudnn: https://sourcegraph.com/github.com/NVIDIA/cudnn-frontend@b372d39879d44c91a8d5b342022e74802b6a8da2/-/blob/include/cudnn_frontend/node/dln.h?L13
The second part of #45