Feature: HiDiffusion integration - #8787
Conversation
|
I might be spoiled by SDXL too much, but SD1.5 did not impress me much. It produces better results than default denoise. Especially when generating whole image. Inpainting is a tricky one. You might want to try different schedulers. Hands are particularly messy at 1.5k px. |
|
I just realised that updated version of HiDiffusion is here: https://github.com/Teriks/dgenerate/tree/master/dgenerate/extras/hidiffusion |
|
You may wish to consider vendoring the code inside the Invoke package, e.g. |
…ps for external dependency
|
JPPhoto
left a comment
There was a problem hiding this comment.
One more that I found:
invokeai/backend/stable_diffusion/hidiffusion_utils.py:114:hidiffusion_patch()never accepts or passes invocation-seededtorch.Generator; vendored window attention therefore reachestorch.rand(1)process-global RNG atinvokeai/backend/hidiffusion/hidiffusion.py:1420. Same seed can produce different images when window attention is enabled. Test by running two identical HiDiffusion window-attention denoises with the same seed while perturbing global Torch RNG between runs.
|
Fixed both modular and legacy denoise paths to create a dedicated torch.Generator seeded from the invocation seed and pass it through hidiffusion_patch() into the vendored window-attention implementation. Added a regression test that runs the real window-attention patch twice with the same seed while perturbing the process-global Torch RNG; both results are identical. |
|
HiDiffusion runtime state could persist on cached UNet modules after the patch was removed. Re-enabling HiDiffusion or changing the bounding-box dimensions could therefore reuse stale Added fix |
JPPhoto
left a comment
There was a problem hiding this comment.
The latest changes have an issue:
invokeai/backend/hidiffusion/hidiffusion.py:2182:remove_hidiffusion()restores classes but never calls_reset_hidiffusion_runtime_state(). If downsampler forward fails after geometry changes at lines 1860-1862 but before restoration, cachedstride,padding, anddilationremain mutated; next HiDiffusion-disabled run uses corrupted convolution geometry. Re-enable reset is too late and skipped when RAU-Net is disabled. Test: forceF.conv2dto raise during patched downsampler forward, exit patch context, then assert geometry is restored before an unpatched forward.
Here's a regression test you can modify and drop in that shows the fault:
from unittest.mock import patch
import pytest
import torch
from invokeai.backend.stable_diffusion.hidiffusion_utils import hidiffusion_patch
class ModelMixin(torch.nn.Module):
"""Minimal cached UNet accepted by HiDiffusion's diffusers type check."""
def __init__(self) -> None:
super().__init__()
self.num_upsamplers = 3
self._num_timesteps = 10
self.block = torch.nn.Conv2d(1, 1, kernel_size=3, stride=2, padding=1)
def test_hidiffusion_teardown_restores_downsampler_geometry_after_forward_error() -> None:
module_keys = {
"down_module_key": ["block"],
"down_module_key_extra": [],
"up_module_key": [],
"up_module_key_extra": [],
"windown_attn_module_key": [],
}
model = ModelMixin()
original_stride = model.block.stride
original_padding = model.block.padding
original_dilation = model.block.dilation
with patch("invokeai.backend.hidiffusion.hidiffusion.sd15_hidiffusion_key", return_value=module_keys):
with hidiffusion_patch(
model,
name_or_path="runwayml/stable-diffusion-v1-5",
apply_window_attn=False,
):
model.info["size"] = (64, 64)
with (
patch(
"invokeai.backend.hidiffusion.hidiffusion.F.conv2d",
side_effect=RuntimeError("injected convolution failure"),
),
pytest.raises(RuntimeError, match="injected convolution failure"),
):
model.block(torch.zeros(1, 1, 16, 16))
# The cached module is now unpatched. A HiDiffusion-disabled generation
# must see its original convolution geometry.
assert model.block.stride == original_stride
assert model.block.padding == original_padding
assert model.block.dilation == original_dilation|
Implemented transactional HiDiffusion teardown. Downsampler geometry is now passed to |
HiDiffusion (invoke-ai#8787) landed in main while this PR was in review, adding five keys to zParamsState after the _version 3 -> 4 bump. Keys added after a bump land in a tier the migration chain cannot reach — a v4 blob matches no branch — so they were seeded by an ad-hoc block inside migrate() instead. That works, but it sits outside applyParamsVersionMigrations(), so the completeness invariant added by this PR cannot see it and reports the five keys as unseeded. Give them zod defaults, the same route the ERNIE-Image and PiD fields take, and drop the now-redundant block: the defaults carry the identical values, and the repair pass covers the blob before the parse either way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
HiDiffusion (invoke-ai#8787) landed in main while this PR was in review, adding five keys to zParamsState after the _version 3 -> 4 bump. Keys added after a bump land in a tier the migration chain cannot reach — a v4 blob matches no branch — so they were seeded by an ad-hoc block inside migrate() instead. That works, but it sits outside applyParamsVersionMigrations(), so the completeness invariant added by this PR cannot see it and reports the five keys as unseeded. Give them zod defaults, the same route the ERNIE-Image and PiD fields take, and drop the now-redundant block: the defaults carry the identical values, and the repair pass covers the blob before the parse either way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ase-upgrade params wipe) (#9408) * fix(ui): pin the conditional half of the Wan v3->v4 seeds The Wan video PR (#9163) added five keys to zParamsState (wanTransformerLowNoise, wanComponentSource, wanVaeModel, wanT5EncoderModel, wanGuidanceScaleLowNoise) while the persisted params schema was still at _version 3, without a version bump or migration seed. The keys are .nullable() with no .default(), which zod treats as required, and migrate() ends with zParamsState.parse() whose failure makes the store silently replace the slice with its initial state. Released v6.13.x builds write v3 blobs without these keys, so any user upgrading from a release to a build containing Wan loses their entire params slice (prompts, prompt history, model selection, dimensions, generation settings) on first launch. Dev machines don't reproduce it because v3 blobs written after the Wan merge already carry the keys. The seeds themselves are no longer this commit's job: they reached main with the FLUX.2 [dev] merge (f10d2a4), together with a field-accurate released-build v3 fixture that fails without them. What main does not cover is the other half of the contract — that the seeds are written with `??` rather than assigned, so a v3 blob from a dev build after the Wan merge keeps the values it already holds instead of having them reset to null. Add that test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ui): seed post-v3 params fields and add a migration safety net Addresses review feedback on #9408. Finding 1 (incomplete fix): the v3->v4 Wan seeds fixed upgrades from v6.13.x, but releases v6.10.0 - v6.12.0 persist _version 2 blobs, and 15 keys added to zParamsState after v3 was cut are required (no .default(), .optional() or .catch()) and seeded nowhere: fluxDype{Preset,Scale, Exponent}, zImageShift, zImageSeedVariance{Enabled,Strength, RandomizePercent}, anima{VaeModel,Qwen3EncoderModel,Scheduler}, klein{VaeModel,Qwen3EncoderModel} and qwenImage{ComponentSource, Quantization,Shift}. Seed them conditionally in the v2->v3 step, so released v2 blobs migrate cleanly and dev-build v2 blobs keep the values they already hold. Verified by running the real migrate() over a blob built from the v6.10.0 release key set: it threw on exactly those 15 paths before this change. Finding 2 (tests can't catch the next occurrence): the fixtures spread getInitialParamsState(), so they carry every current key and are inert against the general defect. Replace them with the top-level zParamsState key sets as actually shipped, read out of the release tags and checked in, one per persisted version still in the wild (v6.10.0 for v2 and v6.13.7 for v3 - each the narrowest key set among the releases writing that version, so a subset of every real blob). Add a schema-completeness test that runs the version steps over each release blob and asserts no key of the current schema is left unhandled, naming the offending keys and the step to fix. It fails on any future required-no-default key added without a seed. Finding 3 (fail-open-and-destroy): a single missing key made zParamsState.parse() throw, and the caller in store.ts falls back to the initial state, wiping prompts, model selection and dimensions with only a log.warn. Add backfillMissingParamsKeys(): after the version steps, fill any key that is absent and that the schema cannot fill itself, and warn with the key names. Narrow by design - a key that is present but invalid still throws, and anything with a .default()/.catch()/.optional() is left to zod. So a forgotten seed now costs one field at its default instead of the user's whole params slice. The completeness test above deliberately bypasses the net so it still fails CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(ui): cover the oldest v2 releases and harden the migration edges Follow-up from an adversarial review of the previous commit. The v2 fixture was not the narrowest v2 release. The earlier survey globbed tags as v6.1*, which silently excluded v6.7.0 - v6.9.0 — four stable releases that also persist _version 2, with only 46 keys against v6.10.0's 52. Six further keys are required-with-no-default and seeded nowhere: fluxScheduler, zImageScheduler, colorCompensation, zImageVaeModel, zImageQwen3EncoderModel and zImageQwen3SourceModel. Verified by running the version steps over a v6.7.0-shaped blob: parse() throws on exactly those six. Seed them in the v2 -> v3 step and replace the fixture with the true narrowest set (v6.7.0, confirmed a strict subset of v6.10.0/v6.11.x/v6.12.0). Add a v6.6.0 (_version 1) fixture too; it is the v6.7.0 set minus positivePromptHistory, which the v1 step already seeds. Also close three edges the safety net did not cover: - The v0 step dereferenced state.dimensions.rect unguarded, so a blob lacking dimensions threw a TypeError straight out of migrate() — the one remaining path that could still wipe the slice. Guard it and let the backfill repair dimensions instead. - The v0 branch tested key presence (!('_version' in state)) while the backfill tests value (!== undefined). A blob with an explicit undefined _version matched no branch, reached the parse and took the slice down. Detect v0 by value so the two agree. - Exclude _version from the backfill loop, so a future change cannot turn it into a version-detection bypass that stamps a blob current without running a single step. Each fix is mutation-checked: reverting any one of them fails at least one test, and the six seeds fail the schema-completeness test. Not covered: v6.2.0a1 - v6.5.1 persist a blob with no _version at all and predate the current dimensions shape, so a faithful fixture can't be built by filtering getInitialParamsState(). Noted in the test file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(ui): close the v4 and v0 gaps in the params migration invariant Addresses the four follow-ups from Pfannkuchensack's second review. The v4 tier was unguarded: the PiD fields landed a day after the _version 3 -> 4 bump, so dev builds from that window persist v4 blobs without them, and a v4 blob matches no branch in the migration chain. Give the four fields zod defaults, matching the ernieImage* precedent set by the other two post-bump additions, and pin a RELEASE_PARAMS_KEYS entry to the bump commit's key set so the invariant covers the tier no version step can reach. Add v0 fixtures. The claimed v0 range was wrong: it spans v6.0.0a1 - v6.6.0rc2, and the oldest builds have no `dimensions` key at all, which no step seeded — the invariant only held there because the safety net caught it. Seed `dimensions` in the v0 step and cover both v0 shapes. Widen the safety net from omissions to any key whose persisted value fails its own field schema, so a `dimensions` the v0 guard left incomplete, or a `model` whose base has since left zBaseModelType, costs that one field instead of the whole slice. This makes true what the guard's comment already claimed. Fix the comments that still described the presence check this branch replaced with a value check. Also close three holes in the tests themselves, all found by mutating the production code and watching nothing fail: the _version guard test never reached the guard (the version steps normalise _version first), the PiD test could not distinguish the new defaults from the safety net backfilling the same values, and the fixtures carried initial values throughout — including `model: null`, so nothing noticed a model being silently cleared. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(ui): give the HiDiffusion params fields zod defaults HiDiffusion (#8787) landed in main while this PR was in review, adding five keys to zParamsState after the _version 3 -> 4 bump. Keys added after a bump land in a tier the migration chain cannot reach — a v4 blob matches no branch — so they were seeded by an ad-hoc block inside migrate() instead. That works, but it sits outside applyParamsVersionMigrations(), so the completeness invariant added by this PR cannot see it and reports the five keys as unseeded. Give them zod defaults, the same route the ERNIE-Image and PiD fields take, and drop the now-redundant block: the defaults carry the identical values, and the repair pass covers the blob before the parse either way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(ui): close three inert spots in the params migration suite Found by mutation-testing this branch's own suite: 116 single-line mutations of the production code, each run against the tests. Three classes of mutation left the suite green. Conditional seeds. Turning `state.X = state.X ?? V` into `state.X = V` went undetected for 21 of the 31 conditional seeds, because only 10 were probed by the two hand-written "preserves ... dev-build" tests. The `??` is the entire point of those lines: the field landed in the schema before the bump that seeds it, so a blob from that window already holds a real user value, and an unconditional assignment resets it. Replace the two tests with a table covering every conditional seed, one probe per key. The table self-validates — each probe must satisfy the field's own schema and must differ from what the step would seed — so a probe that stops discriminating fails rather than going quiet. Verified: seven representative mutations now fail, each naming its own key. Fixture erosion. `buildReleaseBlob` sources values from getInitialParamsState(), so a fixture key that leaves zParamsState is dropped from the blob silently while still sitting in the table, and the fixture stops reproducing the shape it names. That already happened: kleinVaeModel left the schema when the v4 -> v5 step folded it into flux2VaeModel, and it is the input to that fold. Add REMOVED_SCHEMA_KEY_VALUES so removed keys are still reproduced, plus a guard test that fails on any fixture key in neither the schema nor that table. Post-bump defaults. Three mechanisms now write the PiD fields with identical values — the v3 -> v4 seed, the v4 -> v5 seed and the zod default — so any one could be reverted with the other two covering for it, and the test that claims to prove the defaults could not see it. Pin the property directly: every key added after the last bump must satisfy `shape[key].safeParse(undefined)`, which is what lets it survive on a tier that has no migration step. Also assert the version steps leave no key holding a schema-rejecting value, and note in the completeness test what it does not cover: fixture values are initial values, so a tightened *nested* schema still costs a top-level key via `reset` without any fixture noticing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Summary
Added HiDiffusion support across backend and UI. The denoise pipeline can now apply HiDiffusion with optional RAU‑Net and Window Attention toggles, including metadata persistence/recall and graph wiring for SD1/SDXL and the SDXL refiner. The UI exposes aligned advanced switches with informational popovers.
Added HiDiffusion documentation and a “Learn more” link.
Related Issues / Discussions
https://github.com/megvii-research/HiDiffusion
Closes #8780
QA Instructions
Merge Plan
Should merge without issue.
Checklist
What's Newcopy (if doing a release after this PR)