Skip to content

Register SimplifiedLayerNormalization to MS domain #26033

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Register SimplifiedLayerNormalization to MS domain #26033

wants to merge 10 commits into from
+59 −1

Conversation

@justinchuby
Copy link
Contributor

Register SimplifiedLayerNormalization to MS domain. Previously it was incorrectly registered to the ONNX domain, causing onnx models to be invalid.

Fix #7573

@justinchuby
Register SimplifiedLayerNormalization to MS domain
78a411b 
Register SimplifiedLayerNormalization to MS domain. Previously it was incorrectly registered to the ONNX domain, causing onnx models to be invalid.
@justinchuby justinchuby requested a review from MaanavD September 12, 2025 23:27
@snnn snnn requested a review from jywu-msft September 12, 2025 23:37
@justinchuby
Copy link
Contributor Author

justinchuby commented Sep 12, 2025 •
edited
Loading

There are errors

cuda_contrib_kernels.obj : error LNK2001: unresolved external symbol "struct onnxruntime::KernelCreateInfo __cdecl onnxruntime::contrib::cuda::BuildKernelCreateInfo<class onnxruntime::contrib::cuda::kCudaExecutionProvider_SimplifiedLayerNormalization_kMSDomain_ver1_float_float_float>(void)" (??$BuildKernelCreateInfo@VkCudaExecutionProvider_SimplifiedLayerNormalization_kMSDomain_ver1_float_float_float@cuda@contrib@onnxruntime@@@cuda@contrib@onnxruntime@@YA?AUKernelCreateInfo@2@XZ) [E:\_work\_temp\build\RelWithDebInfo\onnxruntime_providers_cuda.vcxproj]

Is it the way I registered the ops?

justinchuby
justinchuby commented Sep 15, 2025
View reviewed changes
onnxruntime/contrib_ops/webgpu/webgpu_contrib_kernels.cc
class ONNX_OPERATOR_KERNEL_CLASS_NAME(kWebGpuExecutionProvider, kMSDomain, 1, SimplifiedLayerNormalization);
class ONNX_OPERATOR_KERNEL_CLASS_NAME(kWebGpuExecutionProvider, kOnnxDomain, 1, SimplifiedLayerNormalization);
class ONNX_OPERATOR_KERNEL_CLASS_NAME(kWebGpuExecutionProvider, kMSDomain, 1, SkipLayerNormalization);
class ONNX_OPERATOR_KERNEL_CLASS_NAME(kWebGpuExecutionProvider, kOnnxDomain, 1, SimplifiedLayerNormalization);
Copy link
Contributor Author

@justinchuby justinchuby Sep 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@fs-eire I found a duplicated entry. Do you think it was intended to be something else?

github-actions[bot]
github-actions bot reviewed Sep 15, 2025
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You can commit the suggested changes from lintrunner.

onnxruntime/contrib_ops/js/js_contrib_kernels.cc
BuildKernelCreateInfo<ONNX_OPERATOR_KERNEL_CLASS_NAME(kJsExecutionProvider, kMSDomain, 1, RotaryEmbedding)>,
BuildKernelCreateInfo<ONNX_OPERATOR_KERNEL_CLASS_NAME(kJsExecutionProvider, kMSDomain, 1,
SkipLayerNormalization)>,
BuildKernelCreateInfo<ONNX_OPERATOR_KERNEL_CLASS_NAME(kJsExecutionProvider, kMSDomain, 1,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
BuildKernelCreateInfo<ONNX_OPERATOR_KERNEL_CLASS_NAME(kJsExecutionProvider, kMSDomain, 1,
BuildKernelCreateInfo<ONNX_OPERATOR_KERNEL_CLASS_NAME(kJsExecutionProvider, kMSDomain, 1,

@fs-eire
Copy link
Contributor

fs-eire commented Oct 22, 2025

Is there any following up for this PR?

@justinchuby
Copy link
Contributor Author

I still want to merge but there’s some build errors I don’t understand. Any pointers you can share?

@fs-eire
Copy link
Contributor

fs-eire commented Oct 22, 2025

maybe merge to latest main and check latest error message (if any)? the current code is a little bit out of date

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@tianleiwu tianleiwu Awaiting requested review from tianleiwu

@MaanavD MaanavD Awaiting requested review from MaanavD

@jywu-msft jywu-msft Awaiting requested review from jywu-msft

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

LayerNormalization operator is not in onnx domain

3 participants

@justinchuby @fs-eire