test: direct unit suite for the opt mode-framework core (56 tests)

[arham766]

What does this PR do?

Type of change: new tests

Part of the unit-coverage initiative in #1902. Direct unit suite (56 tests) for modelopt/torch/opt/conversion.py — the mode-framework core every feature (quantize/prune/distill/nas) rides on. Synthetic mode descriptors live in a throwaway registry with complete teardown (including explicit removal from _ModeRegistryCls._all_registries, whose strong reference makes del unreachable — verified zero global residue and clean combined-process runs with test_chaining/test_mode_registry in both orders).

Coverage: manager construction/is_converted/remove_state; add_mode state-dict structure, ordering, by-reference metadata (load-bearing for the update hooks), next_modes/next_prohibited_modes enforcement with failed-adds-not-recorded, export-stack bookkeeping; load_state_dict replay incl. minor-vs-patch version warning rules, deepcopy isolation, unknown-mode/corrupt-config errors; transfer_state_dict; apply_mode (configs, kwargs broadcast+validation, explicit registry, update_for_new_mode write-back); modelopt_state (save_mode_in_state filtering, update_for_save per call); file and BytesIO save/restore round-trips incl. a real quantize mode.

Deliberate non-overlap map with test_chaining.py / test_load_modelopt_state.py / test_mode_registry.py is documented in the suite; the file-based mto.save()/mto.restore() path had no coverage anywhere.

Three genuine defects documented with NOTE tests (happy to send fixes as follow-ups):

1. modelopt_state() returns aliases of the manager's live state — caller mutation silently corrupts recorded state.
2. apply_mode((ModelClass, args), []) crashes in init_modellike → transfer_state_dict ("vanilla case" per its own comment).
3. apply_mode rejects ModeDescriptor instances with TypeError, contradicting its docstring and the ModeType = ModeDescriptor | str annotation.

(Reviewing also surfaced that tests/unit/torch/opt/test_mode_registry.py's final assertion compares a string against registry objects and is vacuously true — flagged for a follow-up test fix.)

Usage

N/A — tests only.

Testing

56 tests, ~0.4s, hermetic. Adversarial review: 14 expectations independently re-derived, 5/5 seeded mutations killed (each by exactly the tests naming that behavior), no vacuous tests, Windows/Linux-safe.

Before your PR is "Ready for review"

• Is this change backward compatible?: ✅
• If you copied code from any other sources or added a new PIP dependency, did you follow guidance in CONTRIBUTING.md: N/A
• Did you write any new necessary tests?: ✅
• Did you update Changelog?: N/A
• Did you get Claude approval on this PR?: N/A (external contributor)

Additional Information

Issue: #1902

Summary by CodeRabbit

• Tests
  • Added broad unit coverage for model conversion state handling, including state creation, update, removal, and versioning behavior.
  • Verified mode sequencing, export stack rules, and state transfer/replay scenarios.
  • Expanded coverage for save/restore round-trips, in-memory restores, and error handling for invalid or corrupted state.
  • Added checks for configuration and metadata handling, including isolation from caller changes and correct propagation of mode settings.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR adds a new unit test file for the modelopt PyTorch conversion module, covering ModeloptStateManager state bookkeeping, mode sequencing, apply_mode behavior, serialization via modelopt_state, and end-to-end save/restore round-trip persistence, using a synthetic test mode registry and fixtures.

Changes

Conversion State Manager Test Coverage

Layer / File(s)	Summary
Test fixtures and synthetic mode registry tests/unit/torch/opt/test_conversion.py	Defines toy mode descriptors with convert/restore/update hooks and a module-scoped fixture registering/removing them for test isolation.
ModeloptStateManager core bookkeeping tests tests/unit/torch/opt/test_conversion.py	Tests default construction, is_converted for root/non-root, single-state enforcement, remove_state, add_mode ordering/accessors, next_modes/next_prohibited_modes, and export stack checks.
State replay and transfer tests tests/unit/torch/opt/test_conversion.py	Tests load_state_dict replay/version handling/deep-copy isolation, transfer_state_dict semantics, and has_state_for_mode_type validation.
apply_mode and modelopt_state serialization tests tests/unit/torch/opt/test_conversion.py	Tests apply_mode no-op/init-inference/kwargs/broadcasting/registry scoping/update hooks, and modelopt_state structure, versioning, and aliasing/isolation.
End-to-end save/restore round-trip tests tests/unit/torch/opt/test_conversion.py	Tests file/buffer save-restore round-trips, restore_from_modelopt_state source validation and error handling, and a ModelLikeModule construction guard.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Related Issues: None mentioned in the summary.

Related PRs: None mentioned in the summary.

Suggested labels: tests, torch, opt

Suggested reviewers: None specified.

🐰 A rabbit hops through tests anew,
Modes and states in tidy queue,
Save, restore, and check each hook,
A synthetic registry by the book,
All green lights — the tests ring true!

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: a direct unit test suite for the opt mode-framework core.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Security Anti-Patterns	✅ Passed	Only changed file is a unit test; scan found no forbidden security patterns, and no dependencies or production code changed.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

tests/unit/torch/opt/test_conversion.py (1)

177-190: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Fixture registration/teardown is sound.

Module-scoped yield fixture correctly registers synthetic modes once and always tears them down after all tests in the module finish (pytest runs yield-fixture teardown regardless of test outcome). The explicit removal from _ModeRegistryCls._all_registries with an inline comment justifying why __del__ won't fire is a good practice for avoiding global test-state residue.

One minor nit: registry.remove_mode(cls().name) instantiates a throwaway instance of each descriptor class purely to read .name. Since name is a fixed literal per subclass, this could be simplified, but it's inconsequential.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/torch/opt/test_conversion.py` around lines 177 - 190, The fixture
cleanup in tc_modes is fine, but the teardown currently creates a throwaway
instance of each _TC_DESCRIPTOR_CLASSES entry just to read its name. Update the
remove-mode path to use the descriptor’s class-level name source instead of
instantiating the class, keeping the same register_mode/remove_mode flow in
tc_modes and preserving the explicit _ModeRegistryCls._all_registries cleanup.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/unit/torch/opt/test_conversion.py`:
- Around line 177-190: The fixture cleanup in tc_modes is fine, but the teardown
currently creates a throwaway instance of each _TC_DESCRIPTOR_CLASSES entry just
to read its name. Update the remove-mode path to use the descriptor’s
class-level name source instead of instantiating the class, keeping the same
register_mode/remove_mode flow in tc_modes and preserving the explicit
_ModeRegistryCls._all_registries cleanup.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 6b6863e9-c03e-4b81-89ff-141f992047e0

📥 Commits

Reviewing files that changed from the base of the PR and between b96a785 and b49e1a8.

📒 Files selected for processing (1)

• tests/unit/torch/opt/test_conversion.py

[arham766]

Consolidated into #1927 per maintainer feedback in #1902 — the suite was trimmed to only the lines codecov reports uncovered, with parametrization clusters deduplicated. Closing in favor of that PR.