feat(optim): show custom operator domains - #1306
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Qiong Wu (qiowu) (DingmaomaoBJTU)
left a comment
Qiong Wu (qiowu) (DingmaomaoBJTU)
left a comment
There was a problem hiding this comment.
Reviewed the domain propagation through graph collection, diff reporting, CLI rendering, structured optimization output, and produced-operator support. The implementation is backward compatible for default-domain ONNX nodes and all CI checks pass. No blocking findings.
| outputs: tuple[str, ...] | ||
| domain: str = "" | ||
|
|
||
| def qualified_op_type(self) -> str: |
There was a problem hiding this comment.
Centralizing qualification here keeps label(), histograms, and support output consistent. Treating both the empty domain and ai.onnx as default preserves existing output while custom domains become unambiguous.
| table[key] = ( | ||
| node.SerializeToString(), | ||
| NodeRef(node.op_type, node.name, tuple(node.output)), | ||
| NodeRef(node.op_type, node.name, tuple(node.output), node.domain), |
There was a problem hiding this comment.
Good to capture node.domain at collection time rather than trying to recover it later from display strings. The added field has a default, so existing NodeRef(op_type, name, outputs) callers remain source-compatible.
| "name": ref.name, | ||
| "outputs": list(ref.outputs), | ||
| } | ||
| if ref.domain and ref.domain != "ai.onnx": |
There was a problem hiding this comment.
Keeping op_type raw and adding domain only for custom-domain nodes is a clean structured representation and avoids changing the JSON shape for ordinary ONNX operators. Non-blocking coverage suggestion: add an explicit ai.onnx serialization assertion so this backward-compatibility rule is locked down in to_dict() as well as in label().
| result.operators.append( | ||
| ProducedOperatorSupport( | ||
| op_type=ref.op_type, | ||
| op_type=ref.qualified_op_type(), |
There was a problem hiding this comment.
This qualification happens only after support has been correlated by output tensor, so it improves reporting without changing runtime support classification. That separation is important for custom operators and looks correct.
2 participants
Summary
--check-optimoutput, such ascom.microsoft::GeluValidation
uv run pytest tests/unit/optim/test_analysis.py tests/unit/commands/test_optimize_cli.py tests/unit/analyze/test_optim_output.pyuv run ruff check --fix src/winml/modelkit/optim/analysis.py src/winml/modelkit/analyze/optim_output.py tests/unit/optim/test_analysis.py tests/unit/commands/test_optimize_cli.py tests/unit/analyze/test_optim_output.py