Add LoRA loading support for MiniMax-H3 - #14408
Conversation
|
this could be entirely avoided if #14410 would be addressed by the team for future model releases. all of the conversions here are simply counter-productive;
|
sayakpaul
left a comment
There was a problem hiding this comment.
Thanks! Have we tried any of the LoRAs mentioned and inferred with them?
|
|
||
| _lora_loadable_modules = ["transformer", "transformer_ref"] | ||
| transformer_name = TRANSFORMER_NAME | ||
| transformer_ref_name = MINIMAX_H3_TRANSFORMER_REF_NAME |
There was a problem hiding this comment.
Do we have a LoRA with this? If not, do we want to remove it?
There was a problem hiding this comment.
yes, here's a reference: matlod/minimax-h3-turnaround it is a published LoRA made for the ref2va workflow - it turns one reference image into a five-view character turnaround, and its card even warns that it degrades normal t2v. We loaded it into transformer_ref and it works as the model card advertises, but we had to explicitly do it.
The reason both names are explicit: a LoRA file doesn't say which of the two transformers it was trained for, so the loader can't guess. But when you save a ref-partition LoRA with save_lora_weights, the file gets the transformer_ref. prefix, so loading it back when saved with diffusers should land on the right transformer automatically.
This is due to the new workflow logic that YiYi added to this pipeline I think in which each transformer has a function so being able to load on each is kinda necessary imo
| if is_non_diffusers_format: | ||
| state_dict = _convert_non_diffusers_minimax_h3_lora_to_diffusers(state_dict) | ||
|
|
||
| # Every published MiniMax-H3 LoRA is alpha-less and applies as `W + lora_B @ lora_A`, i.e. at an effective |
There was a problem hiding this comment.
It's also interesting to see how pipeline-specific this is. I don't think there's a way to faithfully derive this info just from a canonical state dict. @BenjaminBossan thoughts?
There was a problem hiding this comment.
I don't see how this info could possibly derived from the state_dict. The only way to know this is to already know ahead of time that the checkpoint was trained with r == alpha (or that alpha was multiplied into the weights).
If it helps, we could think about allowing lora_alpha=None in PEFT, in which case we assume that it must be equal to r (it's not quite as easy, e.g. what to do about rsLoRA?). That way, there would no longer be the need to set lora_alpha=r everywhere.
…op generic scale test
Three more published H3 LoRAs did not load. Two were key-layout gaps and the third was the reason neither was noticed: a layout that reaches no module at all used to return without an exception. - musubi-tuner writes one flattened `lora_unet_` name per module. It is un-flattened against the H3 module vocabulary, since `qkv_proj`, `token_refiner`, `final_layer` and the output heads carry underscores that are not path separators, and then goes through the existing kohya path. - one producer publishes its own converter's output: diffusers module names with peft's `.default.` infix left in and no component prefix. The infix is dropped and the prefix added. - a state dict that filters to nothing in both partitions now warns instead of loading as a silent no-op, in the wording `load_lora_adapter` uses for the same situation. A fourth loaded, but at the wrong strength. One producer ships no `.alpha` scalars and records the alpha it trained with in the file's own `__metadata__` instead, under `alpha`. Its 8-step turbo LoRA pairs that entry's 8 with rank 128, an effective scale of 0.0625, so synthesizing `alpha == rank` applied the adapter 16x too strong. That entry is now read as the uniform network alpha. Per-module scalars still win when a file carries both, since the converter has already folded them into the weights, and a non-numeric value is warned about and ignored. `__metadata__` only exists on a file, so `_fetch_state_dict` hands it back on request.
|
heyo, i've addressed the reviewer points, and then additionally the latest commit adds the remaining published H3 LoRA layouts (musubi/kohya flattened keys, unprefixed peft dumps, alpha declared only in safetensors metadata) and normalizes to convention: no-matching-keys warns instead of raising, second partition loads via load_lora_into_transformer_ref. that maximizes compatibility with more in-the-wild LoRA I've seen so far. format tests in tests/lora/test_lora_layers_minimax_h3.py (24) |
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
sayakpaul
left a comment
There was a problem hiding this comment.
Thanks for the further updates. My main concerns:
- The elaborate comments can be simplified and can cut right to the chase to explain what they're doing.
- We don't need state-dict level tests in LoRA testing suite for the state dicts we cannot control explicitly. So, IMO, most of the tests that make use of
lora_state_dict()can be safely removed. Just providing loading and effectiveness support is sufficient I think.
| MiniMax-H3 holds two independently trained DiT partitions in one repository — `transformer/` for the `t2va` and | ||
| `fl2va` workflows, `transformer_ref/` for `ref2va` — and a workflow loads only its own. The two are separate | ||
| checkpoints with nothing tied between them, and their module names are identical, so a LoRA trained against one | ||
| loads without error into the other and silently produces garbage. Nothing in a published H3 LoRA records which | ||
| partition it was trained against, so the routing is explicit: a converted state dict targets `transformer.`, and | ||
| `transformer_ref` is reached either by a `transformer_ref.`-prefixed file (what `save_lora_weights` writes) or by | ||
| passing `load_into_transformer_ref=True`. | ||
|
|
||
| Two things to know about third-party H3 LoRAs. LoRAs trained against a *pruned* checkpoint do not load: pruned | ||
| releases replace the timestep MLP with a small interpolation table, so their AdaLN projections take an 8-wide input | ||
| instead of `time_embed_dim`, and the update cannot be mapped onto the released checkpoint — loading fails with a | ||
| size mismatch naming the module. And most published H3 LoRAs carry no alpha information while applying as `W + | ||
| lora_B @ lora_A`, so mixed-rank files are loaded with `alpha == rank` per module (effective scale exactly 1.0) — | ||
| unless the file records the alpha it was trained with in its own safetensors `__metadata__`, under `alpha`, which | ||
| is then honored for every module as `alpha / rank`. Their updates are also small enough relative to the base | ||
| weights that [`~MiniMaxH3LoraLoaderMixin.fuse_lora`] into bfloat16 discards most of the update — prefer the default | ||
| unfused path. |
There was a problem hiding this comment.
Can we simplify this description? I don't think we need all the gory details here.
|
|
||
| _lora_loadable_modules = ["transformer", "transformer_ref"] | ||
| transformer_name = TRANSFORMER_NAME | ||
| transformer_ref_name = MINIMAX_H3_TRANSFORMER_REF_NAME |
| # One producer publishes its own converter's output: diffusers module names, but with peft's adapter-name | ||
| # infix left in the keys and no component prefix. Neither needs the module-name conversion below, so the | ||
| # infix is dropped and the prefix added here. Gating on the infix the way `QwenImageLoraLoaderMixin` and | ||
| # `ZImageLoraLoaderMixin` do, together with requiring that no key carries a component prefix, keeps this from | ||
| # shadowing a file diffusers itself wrote — `write_lora_layers` never emits `.default.`. | ||
| # | ||
| # The module names really are diffusers' own, not a look-alike basis: that producer ships the same adapter in | ||
| # both encodings, and converting the original-format copy reproduces this one's `lora_B @ lora_A` exactly, to | ||
| # 0.0 relative error on all 312 modules. |
There was a problem hiding this comment.
I am not sure we need an elaborate comment here for this (following the convention of the rest of the LoRA mixins). If we want, we could have a shorter / simpler version of it.
| # ai-toolkit writes the original checkpoint's module names under a `diffusion_model.` prefix, the reference | ||
| # `generate.py` / ComfyUI checkpoints carry no prefix at all, and musubi-tuner flattens them under | ||
| # `lora_unet_`, so the module names are what identifies a non-diffusers file. |
|
|
||
| user_agent = {"file_type": "attn_procs_weights", "framework": "pytorch"} | ||
|
|
||
| state_dict, metadata, file_metadata = _fetch_state_dict( |
There was a problem hiding this comment.
Here I would add a short comment explaining why we need return_file_metadata=True as that deviates from the rest.
There was a problem hiding this comment.
added a comment right above the call - the extra return carries the file's own safetensors __metadata__, which some H3 LoRAs use to record their training alpha
| state_dict[f"{prefix}{source}.lora_B.weight"] = torch.randn(out_features, module_rank) | ||
| return state_dict | ||
|
|
||
| def test_lora_state_dict_conversion(self): |
There was a problem hiding this comment.
We don't need this test (following what we do in the rest).
| ) | ||
| assert all(value.shape[1] == rank or value.shape[0] == rank for value in converted.values()) | ||
|
|
||
| def test_lora_state_dict_conversion_without_a_prefix(self): |
There was a problem hiding this comment.
Same. Community checkpoints are hard to manage. So, just providing loading support (and effectiveness) is enough IMO. So, we can remove this test.
| assert "transformer.transformer_blocks.0.attn.to_k.lora_B.weight" in converted | ||
| assert all(key.startswith("transformer.") for key in converted) | ||
|
|
||
| def test_lora_state_dict_conversion_raises_on_an_unknown_module(self): |
| state_dict[f"lora_unet_{module}.alpha"] = torch.tensor(alpha) | ||
| return state_dict | ||
|
|
||
| def test_lora_state_dict_conversion_flattened_layout(self): |
There was a problem hiding this comment.
Not needed. We just test for the stuff what we expect to write and load from diffusers.
| torch.cat([fused_up[ffn_dim:], fused_up[:ffn_dim]]), | ||
| ) | ||
|
|
||
| def test_load_lora_weights_flattened_layout(self): |
|
Thanks @sayakpaul, did the pass, sorry indeed the docstrings & tests were a little bit on the autopilot of the agent. I'll make sure to be tighter on those next time |
sayakpaul
left a comment
There was a problem hiding this comment.
Much better. Thanks for the patience and iteration here. Thank you!
|
Failing tests are unrelated. |
Adds
MiniMaxH3LoraLoaderMixinsoMiniMaxH3ModularPipelineloads LoRAs through the standardload_lora_weightspath, plus a load time converter for the non diffusers formats in circulation.What loads
diffusion_model.prefixed, fused projections).larryvrh/MiniMax-H3-Turbo-Lora, the 4 step turbo LoRA, in its original unprefixed layout.num_inference_steps=5matches upstream--steps 4; the two scheduler design already covers its dual schedule sampler, so no custom sampler is needed.InstantX/MiniMax-H3-Turbo-Lora-Diffusers, the pre converted mirror, without the manualnetwork_alphasworkaround its card currently requires.Conversion
The converter renames onto
MiniMaxH3Transformer3DModel, splits the fused QKV LoRA (sharedlora_A, row splitlora_B), and swaps the SwiGLUfc1halves from[gate; value]to the converted base's[value; gate]. Verified against ComfyUI's owncalculate_weightand ai-toolkit's ownmerge_outon real base weights: converted factor matrices are bitwise identical after the layout mapping, and effective weights agree to float accumulation noise (4.4e-7 abs over 3.2e9 elements). The output is also key for key identical to InstantX's independent conversion.Because these files carry mixed ranks (64 for attention/FFN, 16 for AdaLN) and no alpha keys, the loader synthesizes
alpha == rankmetadata whenever a state dict brings no alpha information of its own, per component, so every module applies at the intended scale 1.0. Without it the global alpha default mis-scales such adapters (rank 64 modules at 0.25x here); that underlying issue inget_peft_kwargsis general and is fixed separately in #14409.Both transformer partitions are supported and routed by prefix (
transformer,transformer_ref), including pipelines loaded withworkflow="ref2va"where onlytransformer_refexists.Notes
fuse_lora()into bfloat16 is lossy for adapters this small relative to the base weights; the docs note to prefer the default unfused path.