feat(unsloth): support Nemotron-H hybrid MoE fine-tuning on DGX Spark#338
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughAdds prebuilt CUDA extension wheels ( ChangesUnsloth training: prebuilt CUDA wheels + device_map support
Possibly Related PRs
Suggested Reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
docker/Dockerfile.nmp-unsloth-training (1)
130-134: ⚡ Quick winWheel path version inconsistency.
mamba-ssmpath hardcodes version2.3.0whilecausal-conv1duses a full wildcard. If multiplecausal-conv1dversions exist in the wheel directory, the wildcard could match unexpectedly. Consider hardcoding the causal-conv1d version for consistency (e.g.,causal_conv1d-1.5.3-cp312*.whl), or document why version flexibility is intentional here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker/Dockerfile.nmp-unsloth-training` around lines 130 - 134, The causal-conv1d wheel path uses a version wildcard pattern while mamba-ssm hardcodes its version, creating inconsistency that could cause unpredictable wheel selection if multiple versions exist. Modify the causal_conv1d wheel path in the RUN command (currently using `causal_conv1d-*cp312*.whl`) to hardcode a specific version number (e.g., `causal_conv1d-1.5.3-cp312*.whl`) to match the explicit versioning approach used for mamba_ssm-2.3.0-cp312*.whl, ensuring consistent and predictable dependency installation.services/unsloth/src/nmp/unsloth/tasks/file_io/callbacks.py (1)
22-39: ⚡ Quick winCross-service inconsistency: automodel still raises.
The automodel service's
get_percentage(atservices/automodel/src/nmp/automodel/tasks/file_io/callbacks.py:18-45) still raisesValueErroron invalid inputs. This divergence could confuse maintainers when similar functions behave differently across services.Consider aligning both implementations to use the clamping approach.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/unsloth/src/nmp/unsloth/tasks/file_io/callbacks.py` around lines 22 - 39, The automodel service's get_percentage function still raises ValueError on invalid inputs, while the unsloth service's get_percentage function now uses the clamping approach to handle out-of-range values gracefully. Align the automodel service's get_percentage function to match the unsloth implementation by replacing the ValueError raising behavior with the same clamping logic (using max/min to constrain current between 0 and total, and returning 0 when total is invalid), while also adding the debug logging that explains why clamping occurs. This ensures consistent error handling across services and prevents maintainer confusion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/nmp_platform_runner/src/nmp/platform_runner/config/local.yaml`:
- Around line 32-36: The CPU executor configuration in the local.yaml file has a
mismatch between its provider and profile settings. The `provider: cpu` at line
32 is paired with `profile: gpu` at line 33, which is incorrect. Change the
profile value from gpu to cpu to ensure the configuration properly reflects CPU
resources and jobs are scheduled with the correct resource expectations.
- Line 36: The launcher_tool_path in local.yaml uses a relative path that
depends on the repository being available at runtime and the platform starting
from the repo root. Replace the relative path
`./services/core/jobs/jobs-launcher/jobs-launcher` with an absolute path
pointing to the built jobs-launcher binary (e.g.,
`/full/path/to/nemo-platform/services/core/jobs/jobs-launcher/jobs-launcher`),
and ensure the documentation or configuration comments clarify that users must
first run `make build-jobs-launcher` to build the Go binary before the platform
can properly inject the launcher into containers.
---
Nitpick comments:
In `@docker/Dockerfile.nmp-unsloth-training`:
- Around line 130-134: The causal-conv1d wheel path uses a version wildcard
pattern while mamba-ssm hardcodes its version, creating inconsistency that could
cause unpredictable wheel selection if multiple versions exist. Modify the
causal_conv1d wheel path in the RUN command (currently using
`causal_conv1d-*cp312*.whl`) to hardcode a specific version number (e.g.,
`causal_conv1d-1.5.3-cp312*.whl`) to match the explicit versioning approach used
for mamba_ssm-2.3.0-cp312*.whl, ensuring consistent and predictable dependency
installation.
In `@services/unsloth/src/nmp/unsloth/tasks/file_io/callbacks.py`:
- Around line 22-39: The automodel service's get_percentage function still
raises ValueError on invalid inputs, while the unsloth service's get_percentage
function now uses the clamping approach to handle out-of-range values
gracefully. Align the automodel service's get_percentage function to match the
unsloth implementation by replacing the ValueError raising behavior with the
same clamping logic (using max/min to constrain current between 0 and total, and
returning 0 when total is invalid), while also adding the debug logging that
explains why clamping occurs. This ensures consistent error handling across
services and prevents maintainer confusion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d32a186a-53d9-428d-8265-8e81d5aa8312
📒 Files selected for processing (8)
docker-bake.hcldocker/Dockerfile.nmp-unsloth-trainingdocker/unsloth/README.mdpackages/nmp_platform_runner/src/nmp/platform_runner/config/local.yamlplugins/nemo-customizer/src/nemo_customizer/skills/nemo-customizer/references/hyperparameters.mdservices/unsloth/src/nmp/unsloth/schemas.pyservices/unsloth/src/nmp/unsloth/tasks/file_io/callbacks.pyservices/unsloth/src/nmp/unsloth/tasks/training/backends/unsloth_sft.py
|
a522cdd to
9146ab2
Compare
9146ab2 to
926b0f0
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/nmp_platform/config/local.yaml (1)
78-87: ⚡ Quick winRemove redundant
launcher_tool_pathfrom executor entries.Both executor entries specify
launcher_tool_path(lines 82, 87) with the same value already defined inexecutor_defaults.docker(line 95). The executor-level config unnecessarily duplicates the default.♻️ Remove redundant configuration
- provider: cpu profile: gpu backend: docker - config: - launcher_tool_path: ./services/core/jobs/jobs-launcher/jobs-launcher - provider: gpu profile: gpu backend: docker - config: - launcher_tool_path: ./services/core/jobs/jobs-launcher/jobs-launcher🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nmp_platform/config/local.yaml` around lines 78 - 87, Remove the redundant launcher_tool_path configuration from both executor entries (the cpu provider and gpu provider executor blocks) in the config file. The launcher_tool_path value is already defined once in the executor_defaults.docker section, so specifying it individually in each executor's config block creates unnecessary duplication. Delete the launcher_tool_path lines from both the cpu executor entry and the gpu executor entry, keeping only the entries in the shared defaults section.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/nmp_platform/config/local.yaml`:
- Around line 78-87: Remove the redundant launcher_tool_path configuration from
both executor entries (the cpu provider and gpu provider executor blocks) in the
config file. The launcher_tool_path value is already defined once in the
executor_defaults.docker section, so specifying it individually in each
executor's config block creates unnecessary duplication. Delete the
launcher_tool_path lines from both the cpu executor entry and the gpu executor
entry, keeping only the entries in the shared defaults section.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7e3a151c-1e29-4c39-8809-a9a988683e7a
📒 Files selected for processing (9)
docker-bake.hcldocker/Dockerfile.nmp-unsloth-trainingdocker/unsloth/README.mdpackages/nmp_platform/config/local.yamlpackages/nmp_platform_runner/src/nmp/platform_runner/config/local.yamlplugins/nemo-customizer/src/nemo_customizer/skills/nemo-customizer/references/hyperparameters.mdservices/unsloth/src/nmp/unsloth/schemas.pyservices/unsloth/src/nmp/unsloth/tasks/file_io/callbacks.pyservices/unsloth/src/nmp/unsloth/tasks/training/backends/unsloth_sft.py
🚧 Files skipped from review as they are similar to previous changes (7)
- services/unsloth/src/nmp/unsloth/tasks/training/backends/unsloth_sft.py
- plugins/nemo-customizer/src/nemo_customizer/skills/nemo-customizer/references/hyperparameters.md
- services/unsloth/src/nmp/unsloth/schemas.py
- docker/unsloth/README.md
- docker-bake.hcl
- services/unsloth/src/nmp/unsloth/tasks/file_io/callbacks.py
- docker/Dockerfile.nmp-unsloth-training
Enable fine-tuning of hybrid Mamba-2 + MoE models (NVIDIA Nemotron-H,
e.g. Nemotron-3-Nano-30B-A3B) on the Unsloth customization backend, plus
two supporting fixes uncovered bringing the backend up on a unified-memory
GB10 (DGX Spark, arm64/Blackwell).
- Add mamba-ssm + causal-conv1d to the nmp-unsloth-training image, installed
from the same prebuilt cu13.1.1/cp312 wheels as nmp-automodel-base via the
causal-conv1d-wheel / mamba-ssm-wheel bake contexts (versions single-sourced,
no recompile). Nemotron-H remote code imports mamba_ssm at load time.
- Add configurable model.device_map (default {"": 0}). Without an explicit
single-device map, accelerate's device_map="auto" sizes the GPU budget from
CUDA free memory, which on unified-memory parts reports a small dynamic
carve-out and spills layers to CPU -- aborting 4-bit loads.
- Fix file_io download crash: get_percentage now clamps instead of raising when
the live file count exceeds the pre-listing total (off-by-one on sources with
nested directories), so a cosmetic progress value can't fail a multi-GB
download mid-transfer.
- Register cpu/gpu + gpu/gpu docker execution profiles in the local runner
config so the customization backends run out-of-the-box on a local platform.
- Docs: document model.device_map and the 16-bit-LoRA path for hybrid MoE models
in the customizer hyperparameters reference; refresh the unsloth image README
build steps (shared wheels / USE_LOCAL_WHEELS, accurate step list).
Validated end-to-end on DGX Spark: Nemotron-3-Nano-30B-A3B loads and trains in
16-bit LoRA (load_in_4bit=false).
Signed-off-by: Albert Cui <[email protected]>
Signed-off-by: Albert Cui <[email protected]>
926b0f0 to
1cae724
Compare
Enable fine-tuning of hybrid Mamba-2 + MoE models (NVIDIA Nemotron-H, e.g. Nemotron-3-Nano-30B-A3B) on the Unsloth customization backend, plus two supporting fixes uncovered bringing the backend up on a unified-memory GB10 (DGX Spark, arm64/Blackwell).
Validated end-to-end on DGX Spark: Nemotron-3-Nano-30B-A3B loads and trains in 16-bit LoRA (load_in_4bit=false).
Summary by CodeRabbit
device_mapsupport for Unsloth model loading to control GPU placement (including single-GPU defaults and explicit mappings).