Unify BuilderBase and GraphBuilder: shared call_op with feature flags#2910
Open
gramalingam wants to merge 2 commits intomainfrom
Open
Unify BuilderBase and GraphBuilder: shared call_op with feature flags#2910gramalingam wants to merge 2 commits intomainfrom
gramalingam wants to merge 2 commits intomainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2910 +/- ##
==========================================
+ Coverage 72.61% 72.63% +0.01%
==========================================
Files 259 259
Lines 31597 31651 +54
Branches 2973 2980 +7
==========================================
+ Hits 22945 22990 +45
- Misses 7643 7653 +10
+ Partials 1009 1008 -1 ☔ View full report in Codecov by Sentry. |
justinchuby
reviewed
May 7, 2026
Rename and restructure: - Rename OpBuilderBase to BuilderBase - Merge op() and _make_node() into single op() method - Promote _create_op to BuilderBase.call_op() as the core node-creation method Feature flags (BuilderFeature enum): - SCHEMA_PARTITION, CAST_INPUTS, CAST_ATTRIBUTES, INFER_SHAPES, CONSTANT_PROPAGATION - FULL = all flags (used by GraphBuilder) - NONE = no extra processing (used by TapeBuilder) GraphBuilder inherits from BuilderBase: - Shares call_op() with feature-gated hooks - Default hook implementations (_get_schema, _partition_inputs_attributes, _cast_inputs, _cast_attributes, _constant_propagation, _infer_shapes) live in BuilderBase - GraphBuilder only overrides: _promote_constant (cached initializers), _generate_node_name, _adapt_outputs (named outputs), _annotate_node (scope metadata), _qualify_value_name (scope prefixing) Constant promotion in BuilderBase: - _input_to_ir_value with scalar-to-tensor promotion and CastLike - _promote_constant hook: base class uses Constant nodes (no name collisions), GraphBuilder uses cached initializers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
f90ed1f to
6cb4e71
Compare
…27 lint - Remove unused _PYTHON_TYPE_TO_DTYPE from tape_builder.py (builder.py has its own local copy that is actually used) - Remove dead import of _PYTHON_TYPE_TO_DTYPE in builder.py - Add noqa: B027 to _annotate_node (intentional no-op hook, not abstract) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
d2b25f5 to
d3b52dd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extends the
BuilderBase/TapeBuilderinfrastructure (from PR #2905) so thatGraphBuilderinherits fromBuilderBase, sharing a singlecall_op()implementation gated by feature flags.Changes
Class rename and API merge
OpBuilderBase→BuilderBase(no backward-compat alias needed — not yet released)op()and_make_node()methods into a singleop(op_type, *args, **kwargs)ergonomic API_create_optoBuilderBase.call_op()— the single core node-creation methodFeature flags (
BuilderFeatureenum)SCHEMA_PARTITION— partition positional args into inputs vs attributes using op schemaCAST_INPUTS— promote scalars to tensors, match type variablesCAST_ATTRIBUTES— cast attributes using schema info (stub for future)INFER_SHAPES— run shape inference after node creationCONSTANT_PROPAGATION— run constant propagation after node creationFULL= all of the above (used by GraphBuilder)NONE= no extra processing (used by TapeBuilder for rewriter/optimizer/version-converter)GraphBuilder inherits from BuilderBase
GraphBuilder(BuilderBase)withfeatures=BuilderFeature.FULLcall_op()is inherited directly — no override needed_get_schema,_partition_inputs_attributes,_cast_inputs,_cast_attributes) live inBuilderBase_input_to_ir_value— constant promotion with caching_generate_node_name/_adapt_outputs— qualified naming_post_create_node— constant propagation + shape inference_annotate_node— scope metadataExports
BuilderFeatureexported fromonnxscriptpackageTesting