feat(architectures): move the fp8-storage exclusion into a loader-flags facet - #32
Draft
Pfannkuchensack wants to merge 1 commit into
Draft
feat(architectures): move the fp8-storage exclusion into a loader-flags facet#32Pfannkuchensack wants to merge 1 commit into
Pfannkuchensack wants to merge 1 commit into
Conversation
…gs facet
`ModelLoader._should_use_fp8` knew one architecture by name. In the middle of
otherwise architecture-blind logic sat
from invokeai.backend.model_manager.taxonomy import BaseModelType, ModelType
if hasattr(config, "base") and config.base == BaseModelType.ZImage:
return False
Z-Image now declares that itself, as LoaderFlagsFacet(supports_fp8_storage=False),
and the loader asks the registry.
The facet is optional and its defaults describe the ordinary case, so an
architecture that needs no exception declares nothing and the accessor hands back
the defaults. That also covers models with base=Any -- CLIP embedders, T5
encoders -- which reach this function too and are never registered: they get the
defaults rather than an error, exactly as the old equality comparison simply
evaluated to False for them.
Two pieces of dead code go with it. The function-local import was importing from
a module load_default.py already imports at module level (taxonomy, line 26), so
it was incidental rather than cycle-avoidance. And the three hasattr guards
cannot fire: Config_Base.__pydantic_init_subclass__ refuses to create a concrete
config class that does not declare type, base and format with defaults. That
invariant is now asserted in a test rather than assumed, since removing the
guards depends on it.
An audit of the loading path answers the question the plan left open in section 7
-- whether other base-specific loader special cases exist. They do not. The only
other base-keyed code in the load package is dispatch or key construction:
model_loaders/lora.py picks a different state-dict conversion per architecture,
model_loader_registry.py builds a lookup key, and krea2.py's fp8 call keys off the
checkpoint's own dtype rather than the base. Dispatch is not a flag, so none of it
belongs here. LoaderFlagsFacet therefore has exactly one field and one
declaration, and the tests pin that boundary.
Known interaction with two in-flight upstream PRs, flagged deliberately.
invoke-ai#9414 removes this exclusion rather than relocating it: the root
cause was fixed inside invoke-ai#8945 and the branch is obsolete, replaced by an
extra_skip_patterns mechanism reading the model's own
_skip_layerwise_casting_patterns. invoke-ai#9415 gives Anima the same declaration. Both
touch _should_use_fp8, so this conflicts textually, and once they land
supports_fp8_storage has no remaining declaration. Landing this first is a
deliberate choice; see the PR description.
openapi.json is unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.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 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.
Sixth of the series from
.ideas/Backend Modularization Plan.md. No behaviour changes.Two in-flight upstream PRs — invoke-ai/InvokeAI#9414
(fp8 for Z-Image) and #9415 (fp8 for Anima) —
delete the exclusion this PR relocates, rather than moving it. Per invoke-ai#9414 the original root cause
was already fixed inside invoke-ai#8945, so the branch is obsolete; it is replaced by an
extra_skip_patternsmechanism that reads the model's own
_skip_layerwise_casting_patterns.Concretely:
_should_use_fp8inload_default.py(+26/−9), so this conflicts textually;supports_fp8_storagehas no remaining declaration, andLoaderFlagsFacethas no user;
!isZImagefrontend guard this PR's audit identified as a duplicate.Landing this anyway is a deliberate decision. Whoever merges should decide the order — most likely
this facet gets reduced to the dead-code cleanup, or dropped, once the fp8 PRs land.
What it does
ModelLoader._should_use_fp8knew one architecture by name:Z-Image now declares that itself and the loader asks the registry. The facet is optional and its
defaults describe the ordinary case, so an architecture needing no exception declares nothing —
including
base=Anymodels (CLIP embedders, T5 encoders) that reach this function and are neverregistered. They get the defaults, exactly as the old equality comparison simply evaluated to
Falsefor them.Two pieces of dead code go with it
load_default.pyalready imports atmodule level (line 26) — incidental,
not cycle-avoidance.
hasattrguards cannot fire:Config_Base.__pydantic_init_subclass__refuses tocreate a concrete config class that does not declare
type,baseandformatwith defaults.That invariant is now asserted in a test rather than assumed, since removing the guards depends on
it.
The plan's open question, answered
§7 left open whether other base-specific loader special cases exist. They do not. An audit of the
whole load package found
load_default.py:241to be the only per-architecture policy in genericloader code. Everything else base-keyed there is dispatch or key construction:
model_loaders/lora.py:118-191model_loader_registry.py:88-93base-type-formatlookup keymodel_loaders/krea2.py:589-599_should_use_fp8So
LoaderFlagsFacethas exactly one field and one declaration, and the tests pin that boundary.Also found, not fixed here
The same "Z-Image has no fp8 storage" fact exists in three places — the backend plus both
frontends:
frontend/web/.../MainModelDefaultSettings.tsx:59-60,151frontend/webv2/.../DefaultSettingsSection.tsx:274-286, whose comment says outright "Mirrors thebackend's
_should_use_fp8exclusions"A backend facet does not propagate to either until the flag is exposed through the model-config API.
Moot for Z-Image specifically once invoke-ai#9414 lands (it deletes all three), but the shape of the problem
is what PR 3 (capabilities) exists to fix.
Verification
pytest tests/backend/architectures tests/backend/model_manager tests/test_imports.py— 857 passedmypy --follow-imports=silent invokeai/backend/architectures— cleanruff@0.11.2 check .+format --check— cleanopenapi.jsonregenerated and compared normalized — identical.🤖 Generated with Claude Code