Skip to content

Align pattern support with node-level checks (scoped 15-pattern rules)#1020

Open
fangyangci wants to merge 6 commits into
mainfrom
fangyangci/subgraph_pattern
Open

Align pattern support with node-level checks (scoped 15-pattern rules)#1020
fangyangci wants to merge 6 commits into
mainfrom
fangyangci/subgraph_pattern

Conversation

@fangyangci

@fangyangci fangyangci commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Artifact pattern generate:https://github.com/gim-home/ModelKitArtifacts/pull/156

Summary

  • Enable rules checks for the scoped 15 patterns (alternative target patterns + check patterns).
  • For pattern cases with node count > 1, run node-level precheck first; if all nodes are supported, directly use the supported result. To keep pattern check results consistent with split node-level check results on supported cases.
  • Unify and stabilize check_result output via centralized sanitization and backward-compatible import path.

Scope

  • available_ops_all.json has 19 pattern definitions in total.
  • This PR intentionally enables rules checks for the scoped 15 patterns, not all 19.

Consistency Note

  • Helper path without node-level precheck exists in:
    • ModelKitArtifacts/scripts/check_ops_run_helps/debug_case_helper/debug_pattern_help.bat
  • Results between the helper path and the node-level precheck path have been verified as consistent.

Validation

  • uv run ruff check src/winml/modelkit/analyze src/winml/modelkit/pattern tests/unit/analyze
  • uv run mypy -p winml.modelkit
  • uv run pytest tests/unit/analyze/runtime_checker/test_ep_checker.py tests/unit/analyze/test_check_ops.py -q

Comment thread src/winml/modelkit/analyze/runtime_checker/ep_checker.py Fixed
Comment thread src/winml/modelkit/analyze/runtime_checker/ep_checker.py Fixed
@fangyangci fangyangci changed the title Fangyangci/subgraph pattern Refactor rules prefilter to break cyclic imports and stabilize runtime checks Jul 13, 2026
@fangyangci fangyangci force-pushed the fangyangci/subgraph_pattern branch from a06949d to c26b180 Compare July 13, 2026 09:45
@fangyangci fangyangci changed the title Refactor rules prefilter to break cyclic imports and stabilize runtime checks Correct pattern support logic with node-level op precheck Jul 13, 2026
@fangyangci fangyangci changed the title Correct pattern support logic with node-level op precheck Enable pattern rules generation and align pattern support with node-level checks Jul 13, 2026
@fangyangci fangyangci changed the title Enable pattern rules generation and align pattern support with node-level checks Align pattern support with node-level checks (scoped 15-pattern rules) Jul 13, 2026
@fangyangci fangyangci marked this pull request as ready for review July 14, 2026 09:14
@fangyangci fangyangci requested a review from a team as a code owner July 14, 2026 09:14

@DingmaomaoBJTU DingmaomaoBJTU left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the logic statically (didn't run the suite). Overall the rules-first prefilter plus the sys_info / yield_skipped cleanups look coherent — I verified no remaining callers pass the removed sys_info/yield_skipped args, device_type.name is the right value to feed RuntimeCheckerQuery, and the synthetic check_result shape matches the real {compile, run} one. Left inline notes; the main one is a design question about skipping the real EP run purely on node-level rules.

Two nits not worth their own threads: available_ops_all.json gained two trailing blank lines at EOF, and _RulesPrefilterProtocol's method body reads better as ... than pass.

Comment thread src/winml/modelkit/analyze/core/rules_prefilter.py
Comment thread src/winml/modelkit/analyze/core/rules_prefilter.py
Comment thread src/winml/modelkit/analyze/core/rules_prefilter.py Outdated
Comment thread src/winml/modelkit/analyze/pattern/check_patterns.py
Comment thread src/winml/modelkit/analyze/runtime_checker/result_sanitizer.py
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