Clean up base constructor in model builder - #2192
Merged
Merged
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the Python model builder base constructor to move scattered initialization logic into grouped helper methods and introduces EP-specific “expansions” (notably for TRT-RTX) to keep the base builder cleaner.
Changes:
- Refactors base builder initialization: graph-capture handling, RoPE naming cleanup, quant-attrs structure flattening, and helper init hooks (
make_lm_head_init,make_quant_init,make_tied_embeddings_init). - Adds TRT-RTX expansion module and wiring intended to patch/extend builder behavior for that EP.
- Moves some model-specific init logic into subclasses (e.g., Phi block-sparse attention and LM-head masking).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| src/python/py/models/quantized_model.py | Removes CodeQL suppressions and minor style cleanup. |
| src/python/py/models/builders/qwen.py | Updates quant-attrs access pattern and minor node naming tweaks. |
| src/python/py/models/builders/phi.py | Adds Phi-specific init hooks for RoPE, block-sparse attention, and LM-head masking. |
| src/python/py/models/builders/llama.py | Adds LLaMA-specific RoPE scaling init hook. |
| src/python/py/models/builders/expansions/trt_rtx.py | New TRT-RTX expansion mixin intended to provide EP-specific subgraph expansions. |
| src/python/py/models/builders/expansions/init.py | Exports TRT-RTX expansion. |
| src/python/py/models/builders/base.py | Main refactor: constructor cleanup, new init hooks, flattened quant attrs, and expansion initialization. |
| src/python/py/models/builders/init.py | Adds sys.path mutation (intended to support expansion imports). |
| src/python/py/models/builder.py | Updates CLI help text for qmoe block size defaults / supported EPs. |
apsonawane
reviewed
Jun 2, 2026
apsonawane
reviewed
Jun 2, 2026
baijumeswani
approved these changes
Jun 4, 2026
kunal-vaishnavi
added a commit
that referenced
this pull request
Jun 9, 2026
### Description This PR adds a README file in `test/` to explain the folder structure for future contributions. ### Motivation and Context [This PR](#2192) updated the entire `test/` directory structure.
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.
Description
This PR cleans up the constructor in the base class of the model builder.
Motivation and Context
Many PR contributions have polluted the constructor which has made it hard to read and understand. This PR standardizes the attribute initializations + subsequent group initialization helper method as the preferred approach.