Repository navigation
fix(config): surface missing dependencies when resolving _target_ - #4631
Merged
Merged
Conversation
_resolve_target() tries shorter module prefixes whenever import_module() raises ModuleNotFoundError. When the module exists but one of its own imports is missing, it also fell back, and the user saw an unrelated error instead of the missing package. For example, without einops, resolving the BagelBackendConfig target of the bagel SFT example reports that nemo_automodel.components.models has no submodule 'bagel' and that the install may be too old, while listing bagel among available models. Fall back to a shorter prefix only when the missing module is the candidate itself or one of its parents; otherwise re-raise the original ModuleNotFoundError, which names the missing package. Co-authored-by: Claude Opus 5.5 <[email protected]> Signed-off-by: 0z5a <[email protected]>
HuiyingLi
reviewed
Nov 3, 2026
HuiyingLi
left a comment
HuiyingLi
left a comment
Contributor
There was a problem hiding this comment.
Suggestion: Approve
Reviewed b6cb009a39676347f56dec63efab7048fccbf7e5, including both changed files and the config/CLI/model-import consumers. No blocking findings. Checking the structured missing-module name preserves the existing longest-prefix resolution while keeping internal dependency failures intact. This belongs in the shared resolver used by _target_ and *_fn.
Validation on Python 3.12.3:
- All 107 native configuration tests and 17 independent CPU import-boundary tests passed.
- Real temporary modules/packages covered nested class/method and namespace resolution, missing candidate/parent fallback, transitive and sibling dependencies, similarly prefixed package names, unnamed exceptions, generic ImportError, and native CLI target/callback handling.
- The new regression test fails on the actual merge base with only its test overlaid; three normal resolution controls pass.
- With
einopsdeliberately unavailable in an isolated process, the actualBagelBackendConfigimport reportsNo module named 'einops'and retains thebagel/autoencoder.pytraceback. The identical check fails on the old implementation, which instead reports the existingbagelpackage as missing.
These are CPU import/configuration checks; no model construction, downloads, training or GPU execution was needed.
Contributor
|
/ok to test b6cb009 |
akoumpa
approved these changes
Nov 4, 2026
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do ?
When a
_target_module exists but one of its own imports is missing, the error names the wrong thing. For example, withouteinops(it is not a base dependency), resolving the target ofexamples/multimodal_finetune/bagel/bagel_sft.yaml,nemo_automodel.components.models.bagel.configuration.BagelBackendConfig, fails with:The message also suggests the installation may be too old, even though
bagelis in the list it prints. With this PR the same call raisesModuleNotFoundError: No module named 'einops'._resolve_target()tries shorter module prefixes wheneverimportlib.import_module()raisesModuleNotFoundError, including when the missing module is a dependency of the candidate rather than the candidate itself.Changelog
components/config/loader.py,_resolve_target(): fall back to a shorter prefix only when the missing module (e.name) is the candidate or one of its parents; otherwise re-raise the original error. Whene.nameis unset, the fallback is unchanged.tests/unit_tests/config/test_loader.py:test_resolve_target_reports_missing_dependency_of_existing_moduleresolves a temporary module that imports a missing package and expects that package's name in the error.Testing
mainat 613cb7c, CPU, Python 3.12.maintests/unit_tests/config_target_witheinopsblockedruff checkandruff format --checkpass on the changed files.Code provenance
Not applicable.
Before your PR is "Ready for review"
Pre checks:
test_loader.py.Additional Information