mirror of
https://github.com/NVIDIA/Model-Optimizer.git
synced 2026-10-02 03:14:52 +08:00
Chenhan D. YuandClaude Sonnet 4.6
4b9225bee2
## Summary - **Root cause:** When `launch.py --yaml <config>` processes a launcher-format YAML, nemo_run constructs `SlurmConfig` directly from the YAML dict for inline tasks (`task_0`, `task_1`, ...). It strips the unrecognised `_factory_` key and only sets known `SlurmConfig` fields. Since `host` is not in the YAML (it should come from the factory), `SlurmConfig.host` is left as `None`, crashing paramiko at `Connection(host=None)`: ``` TypeError: expected str, bytes or os.PathLike object, not NoneType ``` - **Fix:** In `SandboxPipeline.__post_init__`, after collecting inline tasks, detect `host` is falsy (symptom of dropped `_factory_`) and apply the registered `slurm_factory` as base defaults, overlaying YAML-specified fields (identified by `value != SlurmConfig dataclass default`). - **Scope:** Only triggers when `host` is `None`/`""` and `slurm_factory` is registered. No-op otherwise. Does not affect `task=@` / `pipeline=@` paths. ## Architectural note Three factory resolution paths exist today: 1. `task=@` / `pipeline=@` — nemo_run's `@run.cli.factory` registry ✓ 2. `task_configs` list — `_FACTORY_REGISTRY` via `create_task_from_yaml()` ✓ 3. `--yaml` inline tasks — nemo_run's type system, **bypasses all factory registries** ✗ (fixed here) ## Test plan - [ ] 65/65 unit tests pass (`uv run python3 -m pytest tests/ -v`) - [ ] All pre-commit hooks pass (ruff, mypy, bandit) - [ ] Manually verified: `SandboxPipeline` with `slurm_config.host=None` (simulating nemo_run dropping `_factory_`) now gets `host` filled in from the registered `slurm_factory` 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for reusing an existing SSH connection for Slurm-based launches, reducing repeated login prompts during submissions. * Added a new minimal smoke-test example for hostname checks on CPU-only clusters. * **Bug Fixes** * Improved handling of optional GPU settings so GPU allocation can be left unset when not needed. * Fixed Slurm configuration loading so launch settings are preserved correctly when using YAML-based workflows. * **Tests** * Added coverage for SSH reconnect behavior, Slurm launch overrides, and optional GPU configuration handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Chenhan Yu <chenhany@nvidia.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>