Add SimplifiedLayerNormToRMSNorm surgery#2348
Add SimplifiedLayerNormToRMSNorm surgery#2348unnim-qti wants to merge 4 commits intomicrosoft:mainfrom
Conversation
|
@unnim-qti can you please update this PR? We are going to release new Olive version this Friday and this PR will be included |
63ebd29 to
bd06441
Compare
|
Why not make this a rewrite rule, out of curiosity? |
I considered a rewrite rule, but this transform isn’t purely structural.
|
|
Handling value infos and optional output handling should be straightforward with the onnxscript rewriter. I wonder if you saw any blockers? |
|
I didn’t see any hard blockers in the rewriter, but I followed the existing pattern SimplifiedLayerNormToL2Norm, which use ProtoSurgeon to handle variant‑specific wiring, and optional outputs explicitly. Given that precedent and the comparable complexity here, I kept this transform in the ProtoSurgeon style for the consistency. |
can you review the changes and initiate the pending checks. |
| add_eps_name = self.create_new_name(node_name, op_type, "AddEps") | ||
| add_eps_out = f"{add_eps_name}_out" | ||
|
|
||
| eps_const = numpy_helper.from_array(np.array([eps_value], dtype=np.float32), name=f"{add_eps_name}_const") |
There was a problem hiding this comment.
@unnim-qti, this causes an error with fp16 activations, since eps_const is always fp32 in this surgery:
Type Error: Type parameter (T) of Optype (Add) bound to different types (tensor(float16) and tensor(float) in node (/model/layers.0/input_layernorm/LayerNorm_AddEps).
We should be able to get the datatype for epsilon from the type of ln_input.
Similar situation with pow_const, although Pow is allowed to have inputs of different types, so it's not a hard error unlike for Add.
There was a problem hiding this comment.
@qti-mattsinc , Thanks for pointing this out — I’ve updated the surgery to assign epsilon (and pow_const) to match the ln_input datatype, which resolves the FP16 activation type mismatch in the Add node.
7a1d902 to
4f05344
Compare
|
@jambayk , Could you please re‑initiate the rebuild? The docs build failed due to a 404 Client Error. |
Describe your changes
Checklist before requesting a review
lintrunner -a(Optional) Issue link