Sort DASC modules by explicit name

[kaix-nv]

Summary

Follow-up to #2385 addressing its sole defensive Claude suggestion:

• sort discovered GDN modules with an explicit qualified-name key
• prevent any hypothetical duplicate-name fallback from comparing nn.Module objects

This PR is intentionally stacked on #2385 because repository rules protect a PR head branch after creation.

Validation

• focused DASC suite: 23 passed, 1 absent optional Megatron skip
• DASC plus weight sparsity plus attention sparsity compatibility suite: 134 passed, 1 optional skip
• full pre-commit on the touched file: passed
• prior full-package coverage remains 408/408 statements; this change adds no statements and the line is exercised by the focused suite

Summary by CodeRabbit

• Bug Fixes
  • GDN modules are now returned in a consistent, predictable order.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 419e10df-63c6-45ba-ad81-cbe51d54694e

📥 Commits

Reviewing files that changed from the base of the PR and between 2da09a6 and ca749f3.

📒 Files selected for processing (1)

• modelopt/torch/sparsity/state_sparsity/policy.py

Included review availability: Your plan provides up to 12 included reviews per hour; 6 remain after this review.

---

📝 Walkthrough

Walkthrough

The state sparsity policy now sorts GDN modules explicitly by module name before returning them. This replaces default tuple sorting and makes module ordering deterministic.

Changes

State sparsity policy

Layer / File(s)	Summary
Deterministic GDN module ordering modelopt/torch/sparsity/state_sparsity/policy.py	_get_gdn_modules sorts discovered modules by name before constructing the returned dictionary.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Merge Risk: ⚪ Minimal · up to ca749

The deterministic ordering change is covered by the passing validation results and is ready to merge.

🚥 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: sorting DASC modules by an explicit name. It is concise and specific.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files.
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	PASS. The authoritative diff changes only _get_gdn_modules in policy.py, replacing default tuple sorting with key=lambda item: item[0]. The added line does not load data, execute code, add `# no…

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/dasc-state-sparsity-explicit-sort

---

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

[kaix-nv]

/claude review

[claude]

Claude review summary

Scope: full review — the trigger comment carried no extra scoping. 1 file changed (+1/-1), modelopt/torch/sparsity/state_sparsity/policy.py, reviewed in full along with the surrounding _get_gdn_modules helper.

Findings: CRITICAL: 0, IMPORTANT: 0, SUGGESTION: 0

Analysis

The change replaces dict(sorted(identity_modules)) with dict(sorted(identity_modules, key=lambda item: item[0])).

• Ordering is unchanged. identity_modules is a list[tuple[str, nn.Module]] filtered from model.named_modules(). Since named_modules() yields unique qualified-name paths, the tuple comparison in the old code only ever compared element 0, so the explicit key produces byte-identical ordering. Downstream consumers of the returned dict (horizon analysis in _analyze_gdn_modules, and anything relying on deterministic module iteration order) see no change.
• The defensive intent holds. The old form would fall through to comparing nn.Module objects on a name tie and raise TypeError: '<' not supported between instances of .... The key eliminates that path without adding a branch.
• No composability or compatibility surface touched. Private helper, no mode registration, no modelopt_state schema, no config field, no public export, no signature or default change. Nothing to migrate or version-bump.
• No CHANGELOG.rst entry needed — per the repo guidance, defensive hardening of a code path introduced in the same unreleased cycle is not changelog-worthy.
• Performance: the lambda adds one Python call per discovered GDN module during one-time mode application. Not a hot path.

Risk assessment

Very low. Single-line, order-preserving, and it strictly narrows a latent failure mode. The stated validation (focused DASC suite plus the DASC / weight-sparsity / attention-sparsity compatibility suite) exercises this line.

🤖 Generated with Claude Code

[claude]

Claude review passed — no blocking issues found. LGTM

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-11 02:55 UTC

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.76%. Comparing base (2da09a6) to head (ca749f3).
⚠️ Report is 1 commits behind head on feature/dasc-state-sparsity-validation-helpers.

Additional details and impacted files

@@                                 Coverage Diff                                 @@
##           feature/dasc-state-sparsity-validation-helpers    #2386       +/-   ##
===================================================================================
+ Coverage                                           67.60%   78.76%   +11.16%     
===================================================================================
  Files                                                 548      548               
  Lines                                               64193    64192        -1     
===================================================================================
+ Hits                                                43395    50564     +7169     
+ Misses                                              20798    13628     -7170     

Flag	Coverage Δ
examples-diffusers	20.81% <0.00%> (+0.01%)	⬆️
examples-gpt-oss	13.38% <0.00%> (+<0.01%)	⬆️
examples-hf_ptq	21.78% <0.00%> (+0.01%)	⬆️
examples-llm_distill	13.45% <0.00%> (+<0.01%)	⬆️
examples-llm_eval	17.25% <0.00%> (+0.02%)	⬆️
examples-llm_qat	17.59% <0.00%> (+0.03%)	⬆️
examples-llm_sparsity	15.94% <0.00%> (+<0.01%)	⬆️
examples-megatron_bridge	26.26% <0.00%> (-0.01%)	⬇️
examples-specdec_bench	13.13% <0.00%> (+<0.01%)	⬆️
examples-speculative_decoding	17.67% <0.00%> (+0.02%)	⬆️
examples-torch_onnx	21.82% <0.00%> (+0.01%)	⬆️
examples-torch_trt	15.14% <0.00%> (+<0.01%)	⬆️
gpu	58.38% <0.00%> (+42.83%)	⬆️
regression	15.15% <0.00%> (+<0.01%)	⬆️
unit	57.44% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.