Uh oh!
There was an error while loading. Please reload this page.
Pin nemo_automodel below 0.6 for the fastgen example - #2260
Conversation
0.6.0 removed nemo_automodel.recipes.diffusion.train.is_main_process without a replacement, so dmd2_recipe.py fails to import and every diffusers example test errors at collection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
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 (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe FastGen requirements file changes the ChangesFastGen dependency compatibility
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk:⚪ Minimal · up to This localized dependency pin restores compatibility for the fastgen example, with no actionable merge-blocking risk remaining after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.) Full details: Security Anti-PatternsExplanation PASS. The commit changes only ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@## main #2260 +/- ##
=======================================
Coverage 78.92% 78.92% =======================================
Files 523 523 Lines 60747 60747 =======================================
Hits 47943 47943 Misses 12804 12804
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Uh oh!
There was an error while loading. Please reload this page.
|
What does this PR do?
Type of change: Bug fix
nemo_automodel0.6.0 removednemo_automodel.recipes.diffusion.train.is_main_processwithout a replacement (it was a three-line rank-zero predicate in 0.5.0, and 0.6.0 defines no equivalent anywhere in the package).examples/diffusers/fastgen/dmd2_recipe.pyimports it, so the example's import guard fires and every test intests/examples/diffusers/errors at collection:The requirement was
>=0.4.0,<1.0, so CI picked 0.6.0 as soon as it was published and theonnx (diffusers)job started failing on every PR (e.g. runs 33020467654, 33019418460, 33010815298, 33007613265, 33006944292 — all unrelated branches). Capping at<0.6restores the tested range.Every other
nemo_automodelsymbol the example imports still exists in 0.6.0 (_diffusers.auto_diffusion_pipeline.NeMoAutoDiffusionPipeline,recipes.diffusion.train.TrainDiffusionRecipe, and the fourcomponents.datasets.diffusion.*helpers), sois_main_processis the only blocker; the alternative is defining that predicate locally and widening the cap again, which is worth doing separately if the example is meant to track 0.6.Usage
Testing
Reproduced the break by diffing the published wheels:
is_main_processis defined atnemo_automodel/recipes/diffusion/train.py:692in 0.5.0 and absent from 0.6.0 (grep -rn "def is_main_process"over the unpacked 0.6.0 wheel returns nothing). Confirmed the remaining imported symbols are all still present in 0.6.0.CI on this PR exercises the fix directly: the
onnx (diffusers)job installs from this requirements file and is the job that has been failing.Before your PR is "Ready for review"
CONTRIBUTING.md: N/A — existing dependency, tightened bound.tests/examples/diffusers/suite is what this unblocks.🤖 Generated with Claude Code
Summary by CodeRabbit