Repository navigation
fix(inference): load 64 GB Spark weights lazily - #12633
Conversation
The fastsafetensors recipe exhausted host memory during startup on a physical 64 GB Spark. Use safetensors with lazy loading for this profile while retaining its model, context, and runtime settings. Three automatic, picker, and resume assertions failed before the recipe change. The focused suite now passes 28 tests. Physical hardware completed onboarding, local chat, read/write/exec tools, a 30,037-token prompt, and a model/sandbox restart with at least 21.3 GB host memory available. Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (1)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughThe bounded Spark vLLM recipe now uses ChangesSpark recipe loading
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~5 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The 64 GB Spark recipe switches to lazy safetensors loading, and its pinned runtime supports both options. No actionable merge risk remains in the reviewed changes. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall line coverage in commit fc3b75b in the Show a line coverage summary of the most impacted files.
|
ericksoa
left a comment
ericksoa
left a comment
There was a problem hiding this comment.
Reviewed commit fc3b75b against 1812d69. No blocking findings in either changed path.
The 64 GB Spark recipe selects safetensors with lazy loading through the existing catalog and command materialization. The added assertions exercise automatic, picker, and resume installs through real selection logic. The larger Spark recipe, model and image pins, serving limits, and security settings remain unchanged.
Independent local validation: all 28 tests in vllm-fixed-catalog-install.test.ts and vllm-runtime-selection.test.ts passed. Catalog compilation, catalog validation, Oxfmt, and git diff --check passed. The review checkout has no tracked changes. GitHub reports the commit signature as valid.
The physical Spark results are author-provided evidence; I did not rerun hardware validation. Broader CI and CodeRabbit are still running. This approval records the independent code review and focused validation; it does not claim those pending checks have completed.
2 participants
Outcome
The 64 GB Spark recipe now completes local model startup using lazy safetensors loading. A physical 64 GB Spark passed automatic onboarding, local chat, and read, write, and exec tool calls, including after a restart.
Reason
The original fastsafetensors recipe stalled during startup on this host, with repeated
NV_ERR_NO_MEMORYkernel errors. Available host memory fell to about 19 MB, and the operator stopped the container before it became ready. Docker reportedOOMKilled: false.The loader change resolved this failure in the tested configuration. The precise source of the previous allocation pressure remains unproven.
Related issues
Refs #12502
Changes
--load-format safetensors --safetensors-load-strategy lazyin the existing 64 GB Spark recipe.Verification
npx vitest run --project cli src/lib/inference/vllm-fixed-catalog-install.test.ts src/lib/inference/vllm-runtime-selection.test.ts— 28 tests passed.NODE_OPTIONS=--max-old-space-size=8192 npm run typecheck:cli— passed.npm run catalog:compile,npm run catalog:check,npx oxfmt --check src/lib/inference/vllm-fixed-catalog-install.test.ts, andgit diff --check— passed.1812d698aa4c5b12db4475b878bb6d261ac79a6bplus this recipe change; no memory or profile override.nvidia/Qwen3.6-35B-A3B-NVFP4and the 64 GB profile. Onboarding exited zero and the local inference route was healthy.replayInvalid: trueremained successful.Review notes
Both changed paths are sensitive under the repository policy. Self-review covered commit
fc3b75baeae8a37d0a7236a4e687cdff6819432dand its complete two-file change in NVIDIA/NemoClaw against1812d698aa4c5b12db4475b878bb6d261ac79a6b, including the generated launch command and the physical hardware evidence above. No independent pre-publication review exists; both paths await review.Hardware evidence covers one physical host and one restart. It does not establish prolonged or concurrent-load stability. The hosted curl installer was not tested. Existing swap was retained; no system packages, drivers, or kernel settings were changed for this test.
Signed-off-by: Julie Yaunches jyaunches@nvidia.com
Summary by CodeRabbit
safetensorsloading with a lazy loading strategy, replacing the previous loading format.