Uh oh!
There was an error while loading. Please reload this page.
[.ai] document single-file model layout and "don't reimplement Diffus… - #14048
Merged
Conversation
…ionPipeline" Two conventions that weren't written down, surfaced while reviewing a new model/pipeline port: - models.md: add a "Single-file model layout" section. A model lives in one self-contained `transformer_<name>.py`; reuse shared blocks from normalization.py/attention.py/embeddings.py and inline model-specific variants with `# Copied from` rather than creating per-model `attention_processor_<name>.py` / `block_<name>.py` / `rope_<name>.py` companion files (no precedent for these in the repo). - pipelines.md: add gotcha #7, "Don't reimplement DiffusionPipeline." A subclass adds only pipeline-specific steps; device placement, offloading, execution-device resolution, and module registration already exist on the base class. No custom device/offload manager, no `device=` arg on `__call__`, no `set_<component>` setters. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Drop the implementation-specific examples (Lumina class names, per-model transformer_*.py citations) and state the single-file policy generally: everything (attention, blocks, RoPE, model-specific layers) in one model file, with shared blocks either imported from the common modules or brought in via `# Copied from ... with Old->New`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The positive statement (everything in one model file) already conveys it; drop the companion-module filename list and the editorializing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
State the principle and the canonical base APIs; drop the implementation- specific anti-pattern callouts (enable_*_flag booleans, set_<component> setters, the device= arg, a shadow self.execution_device). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Drop the per-category API bullets; one sentence — don't add device placement, offloading, or component loading/registration logic on a subclass, it's on the base class. Can expand later if needed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dg845
reviewed
Jun 23, 2026
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: dg845 <58458699+dg845@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
DN6 pushed a commit
that referenced
this pull request
Jul 1, 2026
#14048) * [.ai] document single-file model layout and "don't reimplement DiffusionPipeline" Two conventions that weren't written down, surfaced while reviewing a new model/pipeline port: - models.md: add a "Single-file model layout" section. A model lives in one self-contained `transformer_<name>.py`; reuse shared blocks from normalization.py/attention.py/embeddings.py and inline model-specific variants with `# Copied from` rather than creating per-model `attention_processor_<name>.py` / `block_<name>.py` / `rope_<name>.py` companion files (no precedent for these in the repo). - pipelines.md: add gotcha #7, "Don't reimplement DiffusionPipeline." A subclass adds only pipeline-specific steps; device placement, offloading, execution-device resolution, and module registration already exist on the base class. No custom device/offload manager, no `device=` arg on `__call__`, no `set_<component>` setters. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * [.ai] generalize single-file model layout section Drop the implementation-specific examples (Lumina class names, per-model transformer_*.py citations) and state the single-file policy generally: everything (attention, blocks, RoPE, model-specific layers) in one model file, with shared blocks either imported from the common modules or brought in via `# Copied from ... with Old->New`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * [.ai] trim redundant don't-split sentence from single-file section The positive statement (everything in one model file) already conveys it; drop the companion-module filename list and the editorializing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * [.ai] generalize the "don't reimplement DiffusionPipeline" gotcha State the principle and the canonical base APIs; drop the implementation- specific anti-pattern callouts (enable_*_flag booleans, set_<component> setters, the device= arg, a shadow self.execution_device). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * [.ai] pare gotcha #7 down to the principle Drop the per-category API bullets; one sentence — don't add device placement, offloading, or component loading/registration logic on a subclass, it's on the base class. Can expand later if needed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * [.ai] renumber gotchas after merge: component-mutation #7, reimplement #8 main's newly merged "don't modify registered component on the fly" keeps #7; our "don't reimplement DiffusionPipeline" becomes #8 (appended after it). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * [.ai] add blank line between gotchas 7 and 8 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Update .ai/models.md Co-authored-by: dg845 <58458699+dg845@users.noreply.github.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: dg845 <58458699+dg845@users.noreply.github.com>
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
adding two gotcha from review in #14040