Skip to content

Split ONNX opsets for legacy and dynamo export - #2

Closed
quic-amitraj wants to merge 2 commits into
smedhe:smedhe_enable_dynamo_for_causallmfrom
quic:opset-split-legacy-dynamo
Closed

Split ONNX opsets for legacy and dynamo export#2
quic-amitraj wants to merge 2 commits into
smedhe:smedhe_enable_dynamo_for_causallmfrom
quic:opset-split-legacy-dynamo

Conversation

@quic-amitraj

@quic-amitraj quic-amitraj commented Jul 21, 2026

Copy link
Copy Markdown

Stacked On

This PR is stacked on quic#1194 by targeting smedhe:smedhe_enable_dynamo_for_causallm.

Problem

Legacy TorchScript export and dynamo export need different ONNX opsets. The previous code used a single ONNX_EXPORT_OPSET value everywhere, so both export paths, custom-op symbolic registration, ONNXScript function protos, and the dynamo custom translation table all effectively shared the same opset-backed custom op definitions.

For this branch we need:

  • legacy export (dynamo=False) to stay on ONNX opset 17
  • dynamo export (dynamo=True) to use ONNX opset 18
  • QEff custom ops (CtxScatter, CtxGather, CustomRMSNorm, etc.) to emit function bodies built against the matching default-domain opset

Approach

Instead of duplicating ctx_scatter_gather.py, ctx_scatter_gather_cb.py, and rms_norm.py into separate opset-17/opset-18 files, this PR keeps one source definition for each custom op and compiles two ONNXScript variants from it.

The helper in QEfficient/customop/onnxscript_utils.py temporarily binds the module-level ops symbol to the requested default ONNX opset while the ONNXScript decorator compiles the function. The default returned function is the legacy/opset-17 variant, and the dynamo/opset-18 variant is attached to it for explicit lookup. This avoids code duplication while ensuring both export modes get different ONNXScript function protos.

Key Changes

  • Adds explicit constants:
    • ONNX_LEGACY_EXPORT_OPSET = 17
    • ONNX_DYNAMO_EXPORT_OPSET = 18
    • get_onnx_export_opset(dynamo)
  • Updates legacy export paths to pass opset_version=17.
  • Updates dynamo export to pass opset_version=18.
  • Makes CustomOpTransform.apply(..., onnx_export_opset=...) mode-aware so it registers custom symbolics and appends ONNXScript function protos for the selected opset.
  • Passes the selected opset from export into the ONNX transform pipeline.
  • Updates DYNAMO_CUSTOM_OP_TABLE to point at the opset-18 ONNXScript variants, because dynamo uses that table during export before post-export transforms run.
  • Adds a regression test proving the same CustomRMSNorm source can produce opset-17 and opset-18 function protos.

Why This Design

Duplicating the custom-op files would work, but it creates long-term maintenance risk: every custom-op behavior fix would need to be made in two places. This PR centralizes only the opset selection while keeping the custom-op implementation itself single-source. The mode boundary is explicit at export time and transform time.

Signed-off-by: Amit Raj <amitraj@qti.qualcomm.com>
Signed-off-by: Amit Raj <amitraj@qti.qualcomm.com>
module.__dict__["ops"] = previous_ops


def qeff_custom_op(domain: str, version: int):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Add a comment saying get rid of the decorator once, legacy export is deprecated.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

do you think is this the right way to do this?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants