Skip to content

MBridge pruning minor fix for saving pruned NemotronH#887

Merged
kevalmorabia97 merged 1 commit intomainfrom
kmorabia/mbridge-minor-pruning
Feb 13, 2026
Merged

MBridge pruning minor fix for saving pruned NemotronH#887
kevalmorabia97 merged 1 commit intomainfrom
kmorabia/mbridge-minor-pruning

Conversation

@kevalmorabia97
Copy link
Collaborator

@kevalmorabia97 kevalmorabia97 commented Feb 13, 2026

What does this PR do?

Type of change: Bug fix

Testing

Nemotron Nano v2 pruned can be saved

Summary by CodeRabbit

  • Bug Fixes

    • Fixed Hugging Face model loading to properly respect the trust_remote_code parameter during model instantiation.
  • Improvements

    • Enhanced distributed training logging with rank-0 aware warning and logging mechanisms for cleaner, non-redundant output in multi-GPU and multi-node scenarios.

Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 13, 2026

📝 Walkthrough

Walkthrough

The changes introduce rank-0 aware logging improvements in a searcher utility, update a method signature to respect a trust_remote_code flag during model loading, and add documentation comments in example files regarding future validation configuration refactoring.

Changes

Cohort / File(s) Summary
Validation Config Documentation
examples/megatron_bridge/distill.py, modelopt/torch/utils/plugins/mbridge.py
Added TODO comments and commented-out validation config lines indicating planned migration to Nemo 26.04 validation configuration pattern.
Model Loading API Update
examples/megatron_bridge/prune_minitron.py
Updated AutoBridge.from_hf_pretrained() call to include trust_remote_code parameter, enabling the method to respect this flag during bridge instantiation.
Rank-0 Aware Logging
modelopt/torch/opt/searcher.py
Replaced generic warnings and print statements with rank-0 specific helpers (warn_rank_0, print_rank_0) for checkpoint validation and state loading operations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: a minor fix for MBridge pruning to enable saving of pruned NemotronH models, which aligns with the primary functional change in prune_minitron.py.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch kmorabia/mbridge-minor-pruning

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
modelopt/torch/opt/searcher.py (1)

266-267: Nit: save_search_checkpoint still uses bare print instead of print_rank_0.

Line 267 uses print(...) while the rest of this file now uses print_rank_0. The dist.is_master() guard on line 262 makes this safe, but switching to print_rank_0 would be more consistent and would let you drop the redundant master check for the verbose path.

Proposed fix
         if verbose:
-            print(f"Saving searcher state to {checkpoint}...")
+            print_rank_0(f"Saving searcher state to {checkpoint}...")

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Feb 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.74%. Comparing base (95511a0) to head (5a25091).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #887   +/-   ##
=======================================
  Coverage   73.73%   73.74%           
=======================================
  Files         199      199           
  Lines       21165    21163    -2     
=======================================
  Hits        15606    15606           
+ Misses       5559     5557    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

manual_gc_interval=100,
),
# TODO: Replace validation args in train with validation config in nemo:26.04
# validation=ValidationConfig(eval_interval=args.eval_interval, eval_iters=args.eval_iters),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will you implement this in the current PR or a later PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Later. It wont be merged into 26.02 container so will raise error if we do it now but wanted to keep it for reference incase we try to mount latest m-bridge and run the script

@kevalmorabia97 kevalmorabia97 merged commit b43ae64 into main Feb 13, 2026
35 of 37 checks passed
@kevalmorabia97 kevalmorabia97 deleted the kmorabia/mbridge-minor-pruning branch February 13, 2026 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants