feat(studio): add DoRA support to studio#7315
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69049e5c9a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| use_gradient_checkpointing = gradient_checkpointing, | ||
| random_state = config.get("random_seed", 3407), | ||
| use_rslora = config.get("use_rslora", False), | ||
| use_dora = config.get("use_dora", False), |
There was a problem hiding this comment.
When the selected model is an embedding encoder that uses FastSentenceTransformer's fast-encoder path, this new flag is accepted here but unsloth/models/sentence_transformer.py builds LoraConfig directly in that path without forwarding kwargs or use_dora. Those runs will persist use_dora: true while training a plain LoRA adapter, so either plumb use_dora into that LoraConfig or hide/reject DoRA for embedding models until it is supported.
Useful? React with 👍 / 👎.
| gradient_checkpointing: config.gradientCheckpointing, | ||
| use_rslora: config.loraVariant === "rslora", | ||
| use_loftq: config.loraVariant === "loftq", | ||
| use_dora: config.loraVariant === "dora", |
There was a problem hiding this comment.
When a user selects DoRA for LoRA/QLoRA and then switches the method to Full Fine-tune, adapterMethod is false but this still serializes use_dora: true. On Mac/MLX the worker now rejects any use_dora before checking training_type/use_lora, so an otherwise full-finetune run fails with “DoRA is not supported for MLX training yet.” Please gate this on adapterMethod or clear the variant when leaving adapter training.
Useful? React with 👍 / 👎.
|
Thanks for this. I went through the whole change and ran a set of verification tests. It is clean, well scoped plumbing and is close to merge. Summary below. What this doesThreads a Correctness traceI confirmed
Both Codex points are addressed in VerificationStatic: all 81 changed YAMLs parse with a boolean Runtime activation tests on a single GPU, QLoRA where applicable, checking that the resulting adapter is genuinely DoRA (magnitude vectors present, trainable, receiving finite non zero gradients) and that a plain LoRA control has none:
Losses and grad norms stay finite and trend down on both text runs. They are different parameterizations so exact parity is not expected, and the DoRA adapter save round trips with its magnitude tensors intact. Optional follow ups (non blocking)
Nice work. Happy to merge once you are good with the above. |
…umenting use_dora is silently dropped on diffusion
|
Hi, thanks for your response, I noted all three follow ups and corrected them. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8e893c4d2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| value: "dora", | ||
| label: "DoRA", | ||
| desc: t("studio.params.weightDecomposed"), |
There was a problem hiding this comment.
When Studio is running on Apple Silicon, this newly added DoRA option remains selectable because the existing Mac guard only clears/disables loftq, but _run_mlx_training now rejects any use_dora with “DoRA is not supported for MLX training yet.” As a result, Mac users can choose DoRA for an adapter run and get an immediate backend error instead of the option being coerced or disabled like LoftQ.
Useful? React with 👍 / 👎.
| use_gradient_checkpointing = use_gradient_checkpointing, | ||
| random_state = 3407, | ||
| use_rslora = use_rslora, | ||
| use_dora = use_dora, |
There was a problem hiding this comment.
When this path is used with a model that is already a PEFT adapter, FastLanguageModel.get_peft_model takes its pre-wrapped adapter branch and compares r, alpha/dropout, targets, use_rslora, etc., but not use_dora because DoRA is only arriving through kwargs. If a user continues training a plain LoRA adapter with DoRA selected, this call is treated as an identical adapter and skipped, so the run records use_dora: true while still training plain LoRA; add use_dora to the core signature/comparison or reject it for pre-wrapped adapters.
Useful? React with 👍 / 👎.
| import type { BackendModelConfig } from "../api/models-api"; | ||
|
|
||
| export type LoraVariant = "lora" | "rslora" | "loftq"; | ||
| export type LoraVariant = "lora" | "rslora" | "loftq" | "dora"; |
There was a problem hiding this comment.
Once dora is a first-class adapter variant, users can train a DoRA adapter and then choose Studio's GGUF LoRA adapter export path, but export_lora_adapter() still calls save_pretrained_gguf(..., save_method="lora"), which goes through llama.cpp's convert_lora_to_gguf.py LoRA-A/B converter. DoRA adapters include magnitude-vector tensors in addition to lora_A/lora_B, so this export path fails or cannot represent the adapter; hide/reject GGUF LoRA adapter export for DoRA checkpoints unless they are merged and exported as a full GGUF model.
Useful? React with 👍 / 👎.
…port, mismatch now correctly falls through to existing error instead of silently no-opping
Resolves the params-section.tsx conflict by keeping main's Mac-aware disabled-state styling (flex-1, disabled:cursor-not-allowed/opacity-60) and extending it to also cover DoRA, not just LoftQ, since DoRA is likewise rejected by the MLX backend. Also extends the existing stale-selection auto-clear effect (previously LoftQ-only) to reset to "lora" if a persisted/imported config has loraVariant "dora" on Mac.
|
Follow-up: I ran a deeper compatibility and cross-platform pass on this. Short version: the feature is additive and safe for everyone who does not turn DoRA on, but it needs a rebase before merge and there are a few small follow-ups. Does it change existing behaviour?No. What I verified
Suggested follow-ups (none block the concept, but please rebase first)
Nice feature. With the rebase and the MLX guard tweak it looks good to me. |
|
Thanks for the fast turnaround. You already covered the main follow-ups from my last pass: the rebase/merge of main, the mutually-exclusive variant validation with a clear rejection under full finetuning, disabling DoRA on Mac, the GGUF LoRA-export preflight guard, and the diffusion note. That all looks good. I pushed one tiny consistency fix on top: the legacy Good to merge from my side once CI is green on the rebased branch. |
|
Thanks you, happy to help! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 520b18670d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| random_state = args.random_state, | ||
| use_rslora = args.use_rslora, | ||
| loftq_config = args.loftq_config, | ||
| use_dora = args.use_dora, |
There was a problem hiding this comment.
When unsloth-cli.py is invoked on Apple Silicon with --use_dora, this new argument is still forwarded into FastLanguageModel.get_peft_model; on MLX that public shim routes to FastMLXModel.get_peft_model, while the Studio MLX worker in this same change explicitly rejects DoRA as unsupported before training. This leaves the legacy CLI exposing a Mac-only adapter option that cannot run correctly, so guard it on is_mlx (or fail with the same clear error) before calling get_peft_model.
Useful? React with 👍 / 👎.
3 participants
Add DoRA (Weight-Decomposed LoRA) as a training option in Studio
Motivation
Studio exposes LoRA, RS-LoRA, and LoftQ only. DoRA (use_dora=True) has been supported in Unsloth core and PEFT since 2026.04, fused kernels, with some training throughput overhead vs. plain LoRA (consistent with PEFT's documented DoRA cost), never surfaced in Studio.
Changes
doraadded as fourthLoraVariant. New pill, 2×2 grid. Wired through YAML/JSON serialization, model-defaults parsing, run-config override, i18n (12 locales).use_doraadded toTrainingRequest, threaded throughroutes/training.py→training.py→trainer.py/worker.pyinto everyget_peft_model()call site.NotImplementedErrorguard on MLX, mirroring the existing LoftQ guard.use_dora: falseadded to all 81 model-default YAMLs.--use_doraflag inunsloth-cli.pyandunsloth_cli/config.py.No core DoRA implementation changes. Studio/CLI integration only.
Testing
Qwen/Qwen3-0.6B. Confirmeduse_dora: truein the persisted job configtest_training_config_popover_source.pypasses withuse_doraadded to the pinned key list.