Skip to content

Add optional graph-modification report to optimize() - #324

Open
take-cheeze wants to merge 1 commit into
onnx:mainfrom
onnxsim:claude/optimize-graph-modifications-result-nv82ma
Open

Add optional graph-modification report to optimize()#324
take-cheeze wants to merge 1 commit into
onnx:mainfrom
onnxsim:claude/optimize-graph-modifications-result-nv82ma

Conversation

@take-cheeze

Copy link
Copy Markdown
Member

The optimization passes already compute how many positive transforms each pass applies to the graph (CountBasedPassAnalysis), but every layer discarded this information, so optimize() only returned the optimized model.

This threads a per-pass modification report through the whole stack:

  • PassManagerAnalysis now carries a transform_counts map (pass name -> total positive transforms); both GeneralPassManager::run and FixedPointPassManager::run populate it instead of returning an empty analysis.
  • Optimizer::optimize / Optimize / OptimizeFixed accept an optional report out-parameter (defaults to nullptr, fully backward compatible).
  • New nanobind bindings optimize_report / optimize_fixedpoint_report and the path-based optimize_from_path_report / optimize_fixedpoint_from_path_report return the report alongside the model.
  • Python optimize() gains a return_report keyword. When True it returns a (model, report) tuple where report is a dict mapping pass name to the number of graph modifications; the default return type is unchanged.

Add tests covering the tuple return, the counted transform, and the zero-count (pass ran but matched nothing) case.

Claude-Session: https://claude.ai/code/session_01TSYXLexqD5r4Lkg3KfeGZj

The optimization passes already compute how many positive transforms each
pass applies to the graph (CountBasedPassAnalysis), but every layer
discarded this information, so optimize() only returned the optimized model.

This threads a per-pass modification report through the whole stack:

- PassManagerAnalysis now carries a transform_counts map (pass name ->
  total positive transforms); both GeneralPassManager::run and
  FixedPointPassManager::run populate it instead of returning an empty
  analysis.
- Optimizer::optimize / Optimize / OptimizeFixed accept an optional report
  out-parameter (defaults to nullptr, fully backward compatible).
- New nanobind bindings optimize_report / optimize_fixedpoint_report and the
  path-based optimize_from_path_report / optimize_fixedpoint_from_path_report
  return the report alongside the model.
- Python optimize() gains a return_report keyword. When True it returns a
  (model, report) tuple where report is a dict mapping pass name to the
  number of graph modifications; the default return type is unchanged.

Add tests covering the tuple return, the counted transform, and the
zero-count (pass ran but matched nothing) case.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSYXLexqD5r4Lkg3KfeGZj
Signed-off-by: take-cheeze <takechi101010@gmail.com>
@take-cheeze
take-cheeze requested review from a team as code owners July 29, 2026 13:23
@take-cheeze

Copy link
Copy Markdown
Member Author

For maintainers reviewing this: I wanted to add a concrete data point for why the per-pass report is worth having, beyond just observability.

A caller doing repeated/fixed-point optimization has to detect "did the last round change anything" somehow. The naive way is to keep the previous ModelProto around and either serialize+hash it or run MessageDifferencer::Equals against the new one. That's a full-message comparison every round, and it's not cheap: a model's serialized size is dominated by initializer weight data, which most passes (shape inference, most of onnx-optimizer's passes) don't touch on a given round, but a naive comparison still has to read all of it just to prove nothing changed.

We ran into exactly this in onnxsim, where this same patch is merged as onnxsim/optimizer#3 (identical diff to this PR). It's consumed in onnxsim/onnxsim#635: the innermost, most-frequently-run fixed point (OptAndShape = alternating shape inference and this optimizer) used to hash the entire serialized model every round to detect convergence -- expensive on models with large initializers, since it has to read all that weight data even on rounds that only touched a handful of nodes. With transform_counts, that check becomes "did any pass report a nonzero count" instead -- exact, and O(pass count) instead of O(model size). onnxsim/onnxsim#637 made this the default path and has real measured numbers: on mixer_l16_224_in21k (~300 MB of initializers), end-to-end simplification went from 268.7s -> 111.0s on the default path and 342.6s -> 85.1s on the graph-native path, validated against a 90-model regression sweep with 0 regressions.

So this isn't a hypothetical win -- it's load-bearing in a real downstream fixed-point optimizer today.
(I've used claude code to write this)

take-cheeze pushed a commit to onnxsim/optimizer that referenced this pull request Aug 21, 2026
… through them

Stacked on onnx#324 (per-pass transform-count report on optimize()) -- this
extends that same report plumbing to two more entry points.

- Optimizer::optimize(Graph&, report=nullptr) / free functions
  OptimizeGraph/OptimizeGraphFixed(Graph&, names, report=nullptr): run the
  configured passes directly on an in-memory Graph, with no ModelProto <->
  Graph round trip at all, for a C++ caller that already holds a Graph.
  Ports onnx#319 (Optimizer::optimize(Graph&), OptimizeGraph,
  OptimizeGraphFixed) and extends it with the report out-param onnx#324 added
  to the ModelProto-based entry points, so a Graph-native caller gets the
  same "did this pass change anything" signal. Not gated behind
  ONNX_IR_PB_CONVERTER_HAS_CONSUMING_OVERLOADS, since these never touch
  ModelProto at all. The two existing ModelProto-based optimize()
  overloads now delegate to this internally instead of calling
  pass_manager->run() directly, so there is one shared code path.
- A consuming (moving) Optimizer::optimize(ModelProto&, ...) overload
  (also gated behind ONNX_IR_PB_CONVERTER_HAS_CONSUMING_OVERLOADS), for a
  caller about to discard or overwrite its input model, avoiding a copy.
- Extend the CSE/duplicate-initializer BLAKE3-digest caching (introduced
  in the companion "trust a BLAKE3 content digest" PR) with a
  clear_tensor_digest_cache option on the Graph-native optimize() overload
  and OptimizeGraph/OptimizeGraphFixed, plus a
  ClearTensorContentDigestCache() the caller can invoke explicitly:
  a caller that reuses the same resident Graph across several optimize()
  calls (e.g. a fixed-point loop that alternates optimize() with shape
  inference) can now keep tensor digests cached across those calls instead
  of paying to recompute them every round, while every other caller keeps
  the safe default of clearing the cache on each call.
- Add pass-phase profiling instrumentation (ONNXOPTIMIZER_PROFILE_PASS_PHASES
  env var) for PredicateBasedPass's matching vs. modifying phases, and use
  Value::hasUsesInCurrentGraph() (the companion onnx/onnx PR) instead of
  uses().size() in eliminate_deadend/eliminate_common_subexpression's
  hot-path use-count checks, avoiding an O(N) scan of each value's uses
  just to test for zero/one.

Depends on onnx#324 (this branch's parent) and on the companion onnx/onnx PR
for Value::hasUsesInCurrentGraph(). onnx#319 is still open upstream; this
supersedes it by including its content plus the report extension, so it
can be closed as included in this PR if maintainers prefer that ordering,
or this can be rebased onto onnx#319 once it merges instead.

Signed-off-by: take-cheeze <takechi101010@gmail.com>
take-cheeze pushed a commit to onnxsim/optimizer that referenced this pull request Aug 21, 2026
… through them

Stacked on onnx#324 (per-pass
transform-count report on optimize()) -- this extends that same report
plumbing to two more entry points.

- Optimizer::optimize(Graph&, report=nullptr) / free functions
  OptimizeGraph/OptimizeGraphFixed(Graph&, names, report=nullptr): run the
  configured passes directly on an in-memory Graph, with no ModelProto <->
  Graph round trip at all, for a C++ caller that already holds a Graph.
  Ports onnx#319
  (Optimizer::optimize(Graph&), OptimizeGraph, OptimizeGraphFixed) and
  extends it with the report out-param
  onnx#324 added to the ModelProto-based
  entry points, so a Graph-native caller gets the same "did this pass
  change anything" signal. Not gated behind
  ONNX_IR_PB_CONVERTER_HAS_CONSUMING_OVERLOADS, since these never touch
  ModelProto at all. The two existing ModelProto-based optimize()
  overloads now delegate to this internally instead of calling
  pass_manager->run() directly, so there is one shared code path.
- A consuming (moving) Optimizer::optimize(ModelProto&, ...) overload
  (also gated behind ONNX_IR_PB_CONVERTER_HAS_CONSUMING_OVERLOADS), for a
  caller about to discard or overwrite its input model, avoiding a copy.
- Extend the CSE/duplicate-initializer BLAKE3-digest caching (introduced
  in the companion "trust a BLAKE3 content digest" PR,
  onnx/optimizer@main...onnxsim:optimizer:claude/upstream-cse-hashing-overhaul,
  not yet opened as its own PR as of this writing) with a
  clear_tensor_digest_cache option on the Graph-native optimize() overload
  and OptimizeGraph/OptimizeGraphFixed, plus a
  ClearTensorContentDigestCache() the caller can invoke explicitly:
  a caller that reuses the same resident Graph across several optimize()
  calls (e.g. a fixed-point loop that alternates optimize() with shape
  inference) can now keep tensor digests cached across those calls instead
  of paying to recompute them every round, while every other caller keeps
  the safe default of clearing the cache on each call.
- Add pass-phase profiling instrumentation (ONNXOPTIMIZER_PROFILE_PASS_PHASES
  env var) for PredicateBasedPass's matching vs. modifying phases, and use
  Value::hasUsesInCurrentGraph() (the companion onnx/onnx PR,
  onnx/onnx#8346) instead of uses().size() in
  eliminate_deadend/eliminate_common_subexpression's hot-path use-count
  checks, avoiding an O(N) scan of each value's uses just to test for
  zero/one.

Depends on onnx#324 (this branch's
parent) and on onnx/onnx#8346 for
Value::hasUsesInCurrentGraph(). onnx#319
is still open upstream; this supersedes it by including its content plus
the report extension, so it can be closed as included in this PR if
maintainers prefer that ordering, or this can be rebased onto onnx#319 once
it merges instead.

Signed-off-by: take-cheeze <takechi101010@gmail.com>
take-cheeze pushed a commit to onnxsim/optimizer that referenced this pull request Aug 21, 2026
… through them

Stacked on onnx#324 (per-pass
transform-count report on optimize()) -- this extends that same report
plumbing to two more entry points.

- Optimizer::optimize(Graph&, report=nullptr) / free functions
  OptimizeGraph/OptimizeGraphFixed(Graph&, names, report=nullptr): run the
  configured passes directly on an in-memory Graph, with no ModelProto <->
  Graph round trip at all, for a C++ caller that already holds a Graph.
  Ports onnx#319
  (Optimizer::optimize(Graph&), OptimizeGraph, OptimizeGraphFixed) and
  extends it with the report out-param
  onnx#324 added to the ModelProto-based
  entry points, so a Graph-native caller gets the same "did this pass
  change anything" signal. Not gated behind
  ONNX_IR_PB_CONVERTER_HAS_CONSUMING_OVERLOADS, since these never touch
  ModelProto at all. The two existing ModelProto-based optimize()
  overloads now delegate to this internally instead of calling
  pass_manager->run() directly, so there is one shared code path.
- A consuming (moving) Optimizer::optimize(ModelProto&, ...) overload
  (also gated behind ONNX_IR_PB_CONVERTER_HAS_CONSUMING_OVERLOADS), for a
  caller about to discard or overwrite its input model, avoiding a copy.
- Extend the CSE/duplicate-initializer BLAKE3-digest caching (introduced
  in the companion "trust a BLAKE3 content digest" PR,
  onnx/optimizer@main...onnxsim:optimizer:claude/upstream-cse-hashing-overhaul,
  not yet opened as its own PR as of this writing) with a
  clear_tensor_digest_cache option on the Graph-native optimize() overload
  and OptimizeGraph/OptimizeGraphFixed, plus a
  ClearTensorContentDigestCache() the caller can invoke explicitly:
  a caller that reuses the same resident Graph across several optimize()
  calls (e.g. a fixed-point loop that alternates optimize() with shape
  inference) can now keep tensor digests cached across those calls instead
  of paying to recompute them every round, while every other caller keeps
  the safe default of clearing the cache on each call.
- Add pass-phase profiling instrumentation (ONNXOPTIMIZER_PROFILE_PASS_PHASES
  env var) for PredicateBasedPass's matching vs. modifying phases, and use
  Value::hasUsesInCurrentGraph() (the companion onnx/onnx PR,
  onnx/onnx#8346) instead of uses().size() in
  eliminate_deadend/eliminate_common_subexpression's hot-path use-count
  checks, avoiding an O(N) scan of each value's uses just to test for
  zero/one.

Depends on onnx#324 (this branch's
parent) and on onnx/onnx#8346 for
Value::hasUsesInCurrentGraph(). onnx#319
is still open upstream; this supersedes it by including its content plus
the report extension, so it can be closed as included in this PR if
maintainers prefer that ordering, or this can be rebased onto onnx#319 once
it merges instead.

Signed-off-by: take-cheeze <takechi101010@gmail.com>
take-cheeze pushed a commit to onnxsim/optimizer that referenced this pull request Aug 21, 2026
… through them

Stacked on onnx#324 (per-pass
transform-count report on optimize()) -- this extends that same report
plumbing to two more entry points.

- Optimizer::optimize(Graph&, report=nullptr) / free functions
  OptimizeGraph/OptimizeGraphFixed(Graph&, names, report=nullptr): run the
  configured passes directly on an in-memory Graph, with no ModelProto <->
  Graph round trip at all, for a C++ caller that already holds a Graph.
  Ports onnx#319
  (Optimizer::optimize(Graph&), OptimizeGraph, OptimizeGraphFixed) and
  extends it with the report out-param
  onnx#324 added to the ModelProto-based
  entry points, so a Graph-native caller gets the same "did this pass
  change anything" signal. Not gated behind
  ONNX_IR_PB_CONVERTER_HAS_CONSUMING_OVERLOADS, since these never touch
  ModelProto at all. The two existing ModelProto-based optimize()
  overloads now delegate to this internally instead of calling
  pass_manager->run() directly, so there is one shared code path.
- A consuming (moving) Optimizer::optimize(ModelProto&, ...) overload
  (also gated behind ONNX_IR_PB_CONVERTER_HAS_CONSUMING_OVERLOADS), for a
  caller about to discard or overwrite its input model, avoiding a copy.
- Extend the CSE/duplicate-initializer BLAKE3-digest caching (introduced
  in the companion "trust a BLAKE3 content digest" PR,
  onnx/optimizer@main...onnxsim:optimizer:claude/upstream-cse-hashing-overhaul,
  not yet opened as its own PR as of this writing) with a
  clear_tensor_digest_cache option on the Graph-native optimize() overload
  and OptimizeGraph/OptimizeGraphFixed, plus a
  ClearTensorContentDigestCache() the caller can invoke explicitly:
  a caller that reuses the same resident Graph across several optimize()
  calls (e.g. a fixed-point loop that alternates optimize() with shape
  inference) can now keep tensor digests cached across those calls instead
  of paying to recompute them every round, while every other caller keeps
  the safe default of clearing the cache on each call.
- Add pass-phase profiling instrumentation (ONNXOPTIMIZER_PROFILE_PASS_PHASES
  env var) for PredicateBasedPass's matching vs. modifying phases, and use
  Value::hasUsesInCurrentGraph() (the companion onnx/onnx PR,
  onnx/onnx#8346) instead of uses().size() in
  eliminate_deadend/eliminate_common_subexpression's hot-path use-count
  checks, avoiding an O(N) scan of each value's uses just to test for
  zero/one.

Note: Graph::initializers_ stays a plain vector<Tensor> throughout (see
the companion onnx/onnx branch above) -- an earlier revision here also
adapted this pass's initializer loop to a vector<unique_ptr<Tensor>>
storage change, since reverted after a benchmark showed no benefit; see
the parent commit's own log for that measurement.

Depends on onnx#324 (this branch's
parent) and on onnx/onnx#8346 for
Value::hasUsesInCurrentGraph(). onnx#319
is still open upstream; this supersedes it by including its content plus
the report extension, so it can be closed as included in this PR if
maintainers prefer that ordering, or this can be rebased onto onnx#319 once
it merges instead.

Signed-off-by: take-cheeze <takechi101010@gmail.com>
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.

2 participants