Add downstream eval to puzzletron v2 - #2104
Open
grzegorz-k-karch wants to merge 15 commits into
Open
Conversation
Signed-off-by: Grzegorz Karch <gkarch@nvidia.com>
Signed-off-by: Grzegorz Karch <gkarch@nvidia.com>
Signed-off-by: Grzegorz Karch <gkarch@nvidia.com>
Signed-off-by: Grzegorz Karch <gkarch@nvidia.com>
Contributor
|
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 (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughPuzzletron now supports configurable vLLM-backed ChangesPuzzletron downstream evaluation
Estimated code review effort: 5 (Critical) | ~90 minutes Sequence Diagram(s)sequenceDiagram
participant SetupWizard
participant CampaignCompiler
participant DownstreamEvaluationNode
participant PostMIPRunner
participant lmms_eval
participant ArtifactStore
SetupWizard->>CampaignCompiler: configure downstream_evaluation topology and tasks
CampaignCompiler->>DownstreamEvaluationNode: materialize checkpoint-dependent node
DownstreamEvaluationNode->>PostMIPRunner: execute selected checkpoint evaluation
PostMIPRunner->>lmms_eval: launch vLLM evaluation command
lmms_eval-->>PostMIPRunner: return evaluation results
PostMIPRunner->>ArtifactStore: persist results, streams, and summary
Suggested reviewers: Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
Note Docstrings generation - SUCCESS |
coderabbitai Bot
added a commit
that referenced
this pull request
Aug 7, 2026
Docstrings generation was requested by @grzegorz-k-karch. * #2104 (comment) The following files were modified: * `modelopt/torch/puzzletron/benchmarks/aiperf.py` * `modelopt/torch/puzzletron/orchestration/adapters/pool.py` * `modelopt/torch/puzzletron/orchestration/adapters/post_mip.py` * `modelopt/torch/puzzletron/orchestration/compiler.py` * `modelopt/torch/puzzletron/orchestration/controller.py` * `modelopt/torch/puzzletron/orchestration/executors/slurm.py` * `modelopt/torch/puzzletron/orchestration/progress.py` * `modelopt/torch/puzzletron/orchestration/task_launcher.py` * `modelopt/torch/puzzletron/post_mip/builtin.py` * `modelopt/torch/puzzletron/post_mip/reporting.py` * `modelopt/torch/puzzletron/post_mip/runner.py` * `modelopt/torch/puzzletron/stages/pipeline.py` * `modelopt/torch/puzzletron/subblock_stats/calc_subblock_stats.py` * `puzzletron_setup/bundle.py` * `puzzletron_setup/v2/validation.py` * `puzzletron_setup/v2/wizard.py` * `puzzletron_setup/wizard.py`
Docstrings generation was requested by @grzegorz-k-karch. * #2104 (comment) The following files were modified: * `modelopt/torch/puzzletron/benchmarks/aiperf.py` * `modelopt/torch/puzzletron/orchestration/adapters/pool.py` * `modelopt/torch/puzzletron/orchestration/adapters/post_mip.py` * `modelopt/torch/puzzletron/orchestration/compiler.py` * `modelopt/torch/puzzletron/orchestration/controller.py` * `modelopt/torch/puzzletron/orchestration/executors/slurm.py` * `modelopt/torch/puzzletron/orchestration/progress.py` * `modelopt/torch/puzzletron/orchestration/task_launcher.py` * `modelopt/torch/puzzletron/post_mip/builtin.py` * `modelopt/torch/puzzletron/post_mip/reporting.py` * `modelopt/torch/puzzletron/post_mip/runner.py` * `modelopt/torch/puzzletron/stages/pipeline.py` * `modelopt/torch/puzzletron/subblock_stats/calc_subblock_stats.py` * `puzzletron_setup/bundle.py` * `puzzletron_setup/v2/validation.py` * `puzzletron_setup/v2/wizard.py` * `puzzletron_setup/wizard.py` <details> <summary>These files were kept as they were</summary> * `tests/unit/torch/puzzletron/test_aiperf_context_capacity.py` * `tests/unit/torch/puzzletron/test_orchestration_compiler.py` * `tests/unit/torch/puzzletron/test_orchestration_controller.py` * `tests/unit/torch/puzzletron/test_orchestration_executors.py` * `tests/unit/torch/puzzletron/test_orchestration_task_topology.py` * `tests/unit/torch/puzzletron/test_post_mip_runner.py` * `tests/unit/torch/puzzletron/test_setup_bundle.py` * `tests/unit/torch/puzzletron/test_setup_v2_post_mip.py` * `tests/unit/torch/puzzletron/test_setup_v2_state_validation.py` * `tests/unit/torch/puzzletron/test_sparse_runtime_stats.py` </details> <details> <summary>These file types are not supported</summary> * `CHANGELOG.rst` * `examples/puzzletron/configs/families/nemotron3/nano_30b_a3b_bf16/runs/lmms_eval.yaml` </details> <details> <summary>ℹ️ Note</summary><blockquote> CodeRabbit cannot perform edits on its own pull requests yet. </blockquote></details> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
modelopt/torch/puzzletron/benchmarks/aiperf.py (1)
442-456: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick winMake vLLM remote-code trust caller-controlled.
run_aiperf_sweepalways enables execution of custom model and tokenizer code. Addtrust_remote_code: bool = False, append--trust-remote-codeonly when true, and include the resolved value incache_identity.🤖 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 `@modelopt/torch/puzzletron/benchmarks/aiperf.py` around lines 442 - 456, Update run_aiperf_sweep to accept trust_remote_code: bool = False, append --trust-remote-code to server_cmd only when that option is enabled, and include the resolved trust_remote_code value in cache_identity so cache entries distinguish the setting.Sources: Coding guidelines, Path instructions, MCP tools
🧹 Nitpick comments (4)
modelopt/torch/puzzletron/stages/pipeline.py (2)
221-225: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional cleanup: dedented brace and redundant
set_struct.Line 221 closes the
workloadsliteral at 4-space indentation, which does not match the enclosing block. Python accepts this, but it reduces readability. Line 225 callsOmegaConf.set_struct(selected, False), whileclone_hydra_configalready clears struct mode on the clone.♻️ Proposed cleanup
- } + } for raw_workload in workloads.values(): workload = dict(raw_workload or {}) selected = clone_hydra_config(hydra_cfg) - OmegaConf.set_struct(selected, False) stats_cfg = selected.calc_subblock_stats🤖 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 `@modelopt/torch/puzzletron/stages/pipeline.py` around lines 221 - 225, Clean up the workload-processing block by aligning the closing workloads-literal brace with its enclosing indentation and removing the redundant OmegaConf.set_struct call after clone_hydra_config. Preserve the existing clone and workload iteration behavior.
359-359: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a brief comment for the local import.
The new function imports
launch_calc_subblock_statsinside the function body. The guideline permits local imports for circular, optional, or heavy dependencies, but requires a brief explanatory comment. Add one line stating the reason, for example the heavy torch/vLLM import chain.Based on learnings from the coding guidelines: "Keep imports at the top of Python source and test files; use local imports only for justified circular dependencies, optional dependencies, or unusually heavy imports, with a brief explanatory comment."
🤖 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 `@modelopt/torch/puzzletron/stages/pipeline.py` at line 359, Add a brief explanatory comment immediately above the local import of launch_calc_subblock_stats in the affected function, noting that it is kept local because its torch/vLLM dependency chain is unusually heavy.Source: Coding guidelines
modelopt/torch/puzzletron/post_mip/runner.py (1)
728-740: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winResult discovery parses every JSON file under the output directory.
rglob("*.json")reads and parses all JSON belowoutput_path. Whenlog_samplesis enabled, lmms-eval writes per-sample logs there. The wizard setslog_samples: Trueby default. For a largelimitthose files dominate the directory, and each is fully loaded into memory only to be discarded by theresultscheck at line 735.Narrow the search to the aggregated result files, or skip files above a size threshold before parsing.
🤖 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 `@modelopt/torch/puzzletron/post_mip/runner.py` around lines 728 - 740, The _lmms_eval_result_payload function currently parses every JSON file, including large per-sample logs. Narrow candidate discovery to aggregated result files using their known filename/pattern, or apply an appropriate size threshold before json.loads, while preserving selection of the newest valid payload containing a results mapping.tests/unit/torch/puzzletron/test_post_mip_runner.py (1)
229-333: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the
model_argsprecedence and the timeout path.The two execution tests cover the success path and the missing-results path. Two behaviors that this PR introduces stay uncovered:
- A user-supplied
model_argsmapping that also setsmodelortensor_parallel_size. See the finding onmodelopt/torch/puzzletron/post_mip/runner.pylines 615-624.- A
subprocess.TimeoutExpiredraised by the fake. See the finding onmodelopt/torch/puzzletron/post_mip/runner.pylines 799-808.Both are cheap to add with the existing
fake_runpattern and no GPU. Add them alongside the fixes.As per path instructions: "Add lean regression tests for command construction, topology/resource allocation, parsing, failures, and environment behavior."
🤖 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/puzzletron/test_post_mip_runner.py` around lines 229 - 333, Extend the lmms-eval tests around _downstream_evaluation with two lean regressions: verify user-supplied model_args takes precedence over generated model and tensor_parallel_size arguments, and make the subprocess fake raise TimeoutExpired to assert the timeout failure behavior and persisted output handling. Reuse the existing node/source fixtures and fake_run pattern without requiring GPU execution.Source: Path instructions
🤖 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.
Inline comments:
In `@modelopt/torch/puzzletron/post_mip/runner.py`:
- Around line 611-613: Sort _LMMS_EVAL_MODEL_ARG_FIELDS before iterating in the
derived-argument construction so --model_args and the persisted argv remain
deterministic across processes. Preserve the existing settings membership check
and derived[key] assignments.
- Around line 695-696: Define a shared _DEFAULT_LMMS_EVAL_TIMEOUT_SECONDS
constant with value 3600.0 in runner.py, use it as the fallback when both
timeout_seconds and timeout are absent in the command-building logic, and reuse
the same constant in the timeout-reporting path around the existing failure
handling instead of duplicating 3600.
- Around line 615-624: Update the model-argument merge logic around
_model_arg_string so derived checkpoint and topology values override
user-supplied values in both branches. For mapping inputs, replace
setdefault-based merging with precedence for derived entries; for string inputs,
ensure duplicate keys resolve to the derived suffix under lmms-eval’s last-value
behavior. Preserve validation and output formatting while guaranteeing the
realized checkpoint and allocated topology are authoritative.
- Around line 799-808: Wrap the subprocess.run call in the relevant runner flow
with a subprocess.TimeoutExpired handler, decode the exception’s stdout and
stderr bytes, and pass them to _write_lmms_eval_streams before re-raising the
timeout. Preserve the existing successful-result handling unchanged.
In `@modelopt/torch/puzzletron/stages/pipeline.py`:
- Around line 254-296: Unify runtime measurement identity across all three
sites: in modelopt/torch/puzzletron/stages/pipeline.py lines 254-296, update
_has_runtime_measurement to use the shared key builder instead of subset
comparisons; in modelopt/torch/puzzletron/subblock_stats/calc_subblock_stats.py
lines 312-325, apply identical defaults and casts for num_iters,
num_warmup_iters, repeat_block_n_times, max_num_seqs, and granularity on
persisted and requested values; in
tests/unit/torch/puzzletron/test_sparse_runtime_stats.py lines 1159-1179, add
num_iters, num_warmup_iters, repeat_block_n_times, and vllm_args to the root
aggregate fixture.
In `@modelopt/torch/puzzletron/subblock_stats/calc_subblock_stats.py`:
- Around line 281-299: Update the runtime-stats identity extraction logic around
the tuple construction to validate that n_embd, batch_size, prefill_seq_len, and
generation_seq_len are present before converting them with int. Return None for
entries missing any required identity field, allowing
calculate_subblock_stats_for_puzzle_dir to skip them and report missing
identities through its existing path; preserve current handling for complete
entries.
In `@puzzletron_setup/wizard.py`:
- Around line 741-748: Update the lmms-eval task prompt in the relevant wizard
function to validate that the entered task list is non-empty, matching
_downstream_evaluation_setting_prompt behavior, so cleared input is rejected at
the interface. Render sequence defaults by joining list-like values with commas
while preserving string defaults, ensuring defaults produced by
_ask_aiperf_config display as valid task text before the existing parsing at
line 826.
- Around line 1069-1075: Update the downstream evaluation branch around
_ask_downstream_evaluation_config so available_metrics is populated from the
selected tasks’ emitted metric names rather than the hardcoded gsm8k.exact_match
path. Include each task’s aggregation suffix, such as strict-match, so
registered paths exactly match the keys produced by the runner.
In `@tests/unit/torch/puzzletron/test_sparse_runtime_stats.py`:
- Line 1320: Remove the assertion referencing hydra_cfg from
test_runtime_stats_resume_signature_includes_workload_id, since that symbol is
undefined and unrelated to the test. Leave the remaining test behavior
unchanged.
---
Outside diff comments:
In `@modelopt/torch/puzzletron/benchmarks/aiperf.py`:
- Around line 442-456: Update run_aiperf_sweep to accept trust_remote_code: bool
= False, append --trust-remote-code to server_cmd only when that option is
enabled, and include the resolved trust_remote_code value in cache_identity so
cache entries distinguish the setting.
---
Nitpick comments:
In `@modelopt/torch/puzzletron/post_mip/runner.py`:
- Around line 728-740: The _lmms_eval_result_payload function currently parses
every JSON file, including large per-sample logs. Narrow candidate discovery to
aggregated result files using their known filename/pattern, or apply an
appropriate size threshold before json.loads, while preserving selection of the
newest valid payload containing a results mapping.
In `@modelopt/torch/puzzletron/stages/pipeline.py`:
- Around line 221-225: Clean up the workload-processing block by aligning the
closing workloads-literal brace with its enclosing indentation and removing the
redundant OmegaConf.set_struct call after clone_hydra_config. Preserve the
existing clone and workload iteration behavior.
- Line 359: Add a brief explanatory comment immediately above the local import
of launch_calc_subblock_stats in the affected function, noting that it is kept
local because its torch/vLLM dependency chain is unusually heavy.
In `@tests/unit/torch/puzzletron/test_post_mip_runner.py`:
- Around line 229-333: Extend the lmms-eval tests around _downstream_evaluation
with two lean regressions: verify user-supplied model_args takes precedence over
generated model and tensor_parallel_size arguments, and make the subprocess fake
raise TimeoutExpired to assert the timeout failure behavior and persisted output
handling. Reuse the existing node/source fixtures and fake_run pattern without
requiring GPU execution.
🪄 Autofix
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: de470a57-4d2a-4c05-80b0-8cba7c728740
📒 Files selected for processing (35)
CHANGELOG.rstexamples/puzzletron/configs/families/nemotron3/nano_30b_a3b_bf16/runs/lmms_eval.yamlexamples/puzzletron/distributed_eval/run_coordinator.shexamples/puzzletron/distributed_eval/run_depth_coordinator.shexamples/puzzletron/distributed_eval/run_depth_pool.shexamples/puzzletron/distributed_eval/run_replacement_pool.shmodelopt/torch/puzzletron/benchmarks/aiperf.pymodelopt/torch/puzzletron/orchestration/adapters/pool.pymodelopt/torch/puzzletron/orchestration/adapters/post_mip.pymodelopt/torch/puzzletron/orchestration/compiler.pymodelopt/torch/puzzletron/orchestration/controller.pymodelopt/torch/puzzletron/orchestration/executors/slurm.pymodelopt/torch/puzzletron/orchestration/progress.pymodelopt/torch/puzzletron/orchestration/task_launcher.pymodelopt/torch/puzzletron/post_mip/builtin.pymodelopt/torch/puzzletron/post_mip/reporting.pymodelopt/torch/puzzletron/post_mip/runner.pymodelopt/torch/puzzletron/stages/pipeline.pymodelopt/torch/puzzletron/subblock_stats/calc_subblock_stats.pypuzzletron_setup/bundle.pypuzzletron_setup/v2/parallel_validation.pypuzzletron_setup/v2/post_mip.pypuzzletron_setup/v2/validation.pypuzzletron_setup/v2/wizard.pypuzzletron_setup/wizard.pytests/unit/torch/puzzletron/test_aiperf_context_capacity.pytests/unit/torch/puzzletron/test_orchestration_compiler.pytests/unit/torch/puzzletron/test_orchestration_controller.pytests/unit/torch/puzzletron/test_orchestration_executors.pytests/unit/torch/puzzletron/test_orchestration_task_topology.pytests/unit/torch/puzzletron/test_post_mip_runner.pytests/unit/torch/puzzletron/test_setup_bundle.pytests/unit/torch/puzzletron/test_setup_v2_post_mip.pytests/unit/torch/puzzletron/test_setup_v2_state_validation.pytests/unit/torch/puzzletron/test_sparse_runtime_stats.py
Contributor
Fixes Applied SuccessfullyFixed 5 file(s) based on 9 unresolved review comments. A stacked PR containing fixes has been created.
Time taken: |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## feature/puzzletron_v2 #2104 +/- ##
=========================================================
- Coverage 54.76% 54.58% -0.18%
=========================================================
Files 704 704
Lines 90869 91193 +324
=========================================================
+ Hits 49761 49775 +14
- Misses 41108 41418 +310
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:
|
This stacked PR contains CodeRabbit auto-fixes for #2104. **Files modified:** - `modelopt/torch/puzzletron/post_mip/runner.py` - `modelopt/torch/puzzletron/stages/pipeline.py` - `modelopt/torch/puzzletron/subblock_stats/calc_subblock_stats.py` - `puzzletron_setup/wizard.py` - `tests/unit/torch/puzzletron/test_sparse_runtime_stats.py` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
modelopt/torch/puzzletron/post_mip/runner.py (1)
681-684: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPut derived model arguments last for string
model_args.At Line 684, the raw string follows
derived. Repeated keys in rawmodel_argscan override the realized checkpoint and allocated topology. This disagrees with the mapping branch at Lines 687-689.Proposed fix
- return ",".join(part for part in (suffix, prefix) if part) + return ",".join(part for part in (prefix, suffix) if part)🤖 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 `@modelopt/torch/puzzletron/post_mip/runner.py` around lines 681 - 684, Update the string-handling branch in _model_arg_string so the raw model arguments precede the derived arguments, matching the mapping branch and ensuring derived checkpoint and topology values take precedence for duplicate keys.
🤖 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.
Duplicate comments:
In `@modelopt/torch/puzzletron/post_mip/runner.py`:
- Around line 681-684: Update the string-handling branch in _model_arg_string so
the raw model arguments precede the derived arguments, matching the mapping
branch and ensuring derived checkpoint and topology values take precedence for
duplicate keys.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d49d16ee-f253-4f33-899e-44f9f7a9bb3d
📒 Files selected for processing (21)
modelopt/torch/puzzletron/benchmarks/aiperf.pymodelopt/torch/puzzletron/orchestration/adapters/pool.pymodelopt/torch/puzzletron/orchestration/adapters/post_mip.pymodelopt/torch/puzzletron/orchestration/compiler.pymodelopt/torch/puzzletron/orchestration/controller.pymodelopt/torch/puzzletron/orchestration/executors/slurm.pymodelopt/torch/puzzletron/orchestration/progress.pymodelopt/torch/puzzletron/orchestration/task_launcher.pymodelopt/torch/puzzletron/post_mip/builtin.pymodelopt/torch/puzzletron/post_mip/reporting.pymodelopt/torch/puzzletron/post_mip/runner.pymodelopt/torch/puzzletron/stages/pipeline.pymodelopt/torch/puzzletron/subblock_stats/calc_subblock_stats.pypuzzletron_setup/bundle.pypuzzletron_setup/v2/validation.pypuzzletron_setup/v2/wizard.pypuzzletron_setup/wizard.pytests/unit/torch/puzzletron/test_orchestration_controller.pytests/unit/torch/puzzletron/test_orchestration_executors.pytests/unit/torch/puzzletron/test_setup_v2_state_validation.pytests/unit/torch/puzzletron/test_sparse_runtime_stats.py
🚧 Files skipped from review as they are similar to previous changes (19)
- puzzletron_setup/v2/validation.py
- tests/unit/torch/puzzletron/test_setup_v2_state_validation.py
- modelopt/torch/puzzletron/orchestration/executors/slurm.py
- modelopt/torch/puzzletron/orchestration/adapters/post_mip.py
- modelopt/torch/puzzletron/orchestration/progress.py
- modelopt/torch/puzzletron/orchestration/controller.py
- modelopt/torch/puzzletron/post_mip/builtin.py
- puzzletron_setup/bundle.py
- tests/unit/torch/puzzletron/test_orchestration_executors.py
- modelopt/torch/puzzletron/orchestration/adapters/pool.py
- modelopt/torch/puzzletron/orchestration/compiler.py
- modelopt/torch/puzzletron/orchestration/task_launcher.py
- puzzletron_setup/wizard.py
- modelopt/torch/puzzletron/benchmarks/aiperf.py
- modelopt/torch/puzzletron/stages/pipeline.py
- modelopt/torch/puzzletron/post_mip/reporting.py
- puzzletron_setup/v2/wizard.py
- tests/unit/torch/puzzletron/test_sparse_runtime_stats.py
- modelopt/torch/puzzletron/subblock_stats/calc_subblock_stats.py
Signed-off-by: Grzegorz Karch <gkarch@nvidia.com>
Signed-off-by: Grzegorz Karch <gkarch@nvidia.com>
Signed-off-by: Grzegorz Karch <gkarch@nvidia.com>
Signed-off-by: Grzegorz Karch <gkarch@nvidia.com>
Signed-off-by: Grzegorz Karch <gkarch@nvidia.com>
Signed-off-by: Grzegorz Karch <gkarch@nvidia.com>
Signed-off-by: Grzegorz Karch <gkarch@nvidia.com>
grzegorz-k-karch
requested review from
kevalmorabia97
and removed request for
a team
August 8, 2026 18:06
Contributor
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
puzzletron_setup/v2/wizard.py (1)
4151-4170: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReturn
BACKimmediately after each prompt.Lines 4151-4168 run three prompts before line 4169 inspects the results. If the user selects back at the
limitprompt, the wizard still asks forbatch_sizeandtimeoutbefore it returnsBACK._serving_setting_promptat lines 4059-4093 checks each answer right after the prompt. Match that behavior.🛠️ Proposed fix
limit = session.integer( f"{prefix}.limit", "lmms-eval sample limit:", default=int(defaults.get("limit", 128)), minimum=1, ) + if limit is BACK: + return BACK batch_size = session.integer( f"{prefix}.batch_size", "lmms-eval batch size:", default=int(defaults.get("batch_size", 1)), minimum=1, ) + if batch_size is BACK: + return BACK timeout = session.integer( f"{prefix}.timeout_seconds", "Per-candidate lmms-eval timeout (seconds):", default=int(defaults.get("timeout_seconds", 3600)), minimum=1, ) - if BACK in (limit, batch_size, timeout): + if timeout is BACK: return BACK🤖 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 `@puzzletron_setup/v2/wizard.py` around lines 4151 - 4170, Update the prompt flow around the limit, batch_size, and timeout assignments to check for BACK immediately after each session.integer call and return it before showing the next prompt, matching _serving_setting_prompt behavior. Remove the deferred combined check after all three prompts.
♻️ Duplicate comments (1)
puzzletron_setup/wizard.py (1)
750-753: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe task prompt still lacks validation and sequence-default rendering.
Line 752 applies
str()to the default. Ifdefaults["tasks"]is a list, the prompt renders"['ifeval', 'gsm8k']", and line 835 produces corrupted task names. The prompt also accepts empty input, which yieldstasks: []and fails later in_configured_lmms_eval_tasks. The v2 prompt_downstream_evaluation_setting_prompthandles both cases atpuzzletron_setup/v2/wizard.pylines 4132-4147.🛠️ Proposed fix
defaults = defaults or {} + raw_default_tasks = defaults.get("tasks", "ifeval,gsm8k") + default_tasks = ( + raw_default_tasks + if isinstance(raw_default_tasks, str) + else ",".join(str(item) for item in raw_default_tasks) + ) + + def validate_tasks(value: str) -> bool | str: + return ( + True + if [item for item in value.split(",") if item.strip()] + else "Enter at least one lmms-eval task." + ) + tasks = prompts.text( "lmms-eval tasks (comma-separated):", - default=str(defaults.get("tasks", "ifeval,gsm8k")), + default=default_tasks, + validate=validate_tasks, )🤖 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 `@puzzletron_setup/wizard.py` around lines 750 - 753, Update the lmms-eval tasks prompt in the wizard flow to render sequence defaults as comma-separated task names rather than Python list syntax, matching _downstream_evaluation_setting_prompt. Add validation that rejects empty input, ensuring the resulting tasks value cannot become an empty list before _configured_lmms_eval_tasks processes it.
🧹 Nitpick comments (2)
modelopt/torch/puzzletron/post_mip/runner.py (1)
562-570: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd
--modeland--batch_sizeto the reserved extra-argument flags.The runner always emits
--modelat line 778 and--batch_sizeat line 784.lmms-evaluses argparse, so a later duplicate flag inextra_argswins. A config that setsextra_args: ["--model", "hf"]therefore replaces the vLLM backend while--model_argsstill carries the vLLM topology keys, and the run fails or evaluates a different backend than the recorded topology.🛡️ Proposed hardening
_LMMS_EVAL_RESERVED_EXTRA_ARG_FLAGS = frozenset( { + "--model", "--model_args", "--model-args", "--output_path", "--output-path", "--tasks", + "--batch_size", + "--batch-size", } )🤖 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 `@modelopt/torch/puzzletron/post_mip/runner.py` around lines 562 - 570, Update the _LMMS_EVAL_RESERVED_EXTRA_ARG_FLAGS set to include --model and --batch_size alongside the existing reserved flags, preventing extra_args from overriding the runner-emitted backend and batch size arguments.tests/unit/torch/puzzletron/test_post_mip_runner.py (1)
230-278: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
pytest.raisesandpytest.mark.parametrizefor these rejection tests.The manual
try/except/elsewithraise AssertionErrorreimplementspytest.raises. The in-test loop also hides which case failed. This pattern repeats in the four tests at lines 518, 565, and 618.♻️ Proposed refactor for one test
-def test_lmms_eval_command_rejects_reserved_model_args(tmp_path): - cases = ( - ({"model": "/ckpts/wrong"}, "model"), - ("dtype=bfloat16,tensor_parallel_size=1", "tensor_parallel_size"), - ) - for model_args, expected in cases: - try: - runner._lmms_eval_command( - { - "tasks": ["ifeval"], - "topology": {"gpu_group_size": 1}, - "model_args": model_args, - }, - checkpoint="/ckpts/candidate", - output_path=tmp_path / "results", - ) - except ValueError as error: - message = str(error) - else: - raise AssertionError("expected reserved lmms-eval model_args to fail") - - assert "reserved lmms-eval model arguments" in message - assert expected in message +@pytest.mark.parametrize( + ("model_args", "expected"), + [ + ({"model": "/ckpts/wrong"}, "model"), + ("dtype=bfloat16,tensor_parallel_size=1", "tensor_parallel_size"), + ], +) +def test_lmms_eval_command_rejects_reserved_model_args(tmp_path, model_args, expected): + with pytest.raises(ValueError, match="reserved lmms-eval model arguments") as error: + runner._lmms_eval_command( + { + "tasks": ["ifeval"], + "topology": {"gpu_group_size": 1}, + "model_args": model_args, + }, + checkpoint="/ckpts/candidate", + output_path=tmp_path / "results", + ) + assert expected in str(error.value)🤖 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/puzzletron/test_post_mip_runner.py` around lines 230 - 278, Refactor the reserved-argument rejection tests around _lmms_eval_command to use pytest.mark.parametrize for individual cases and pytest.raises(ValueError) for the expected failures, asserting the error message within each case. Apply the same pattern to the repeated rejection tests around lines 518, 565, and 618, preserving each case’s expected message fragments.Source: Coding guidelines
🤖 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.
Inline comments:
In `@examples/puzzletron/docs/post_mip_pipeline.md`:
- Around line 136-138: Update the downstream_evaluation description to state
that it invokes python -m lmms_eval via subprocess without a shell, rather than
saying it “shells out.” Clarify that command_prefix and extra_args are passed as
argv elements and must not contain shell syntax.
In `@modelopt/torch/puzzletron/post_mip/runner.py`:
- Around line 708-710: Make the loop over _LMMS_EVAL_MODEL_ARG_FIELDS
deterministic by iterating its keys in a stable order, such as sorted order,
before populating derived. Preserve the existing filtering and value assignment
so _model_arg_string, persisted argv, and the documented command determinism
remain stable for identical configuration.
- Around line 822-823: Define a shared _DEFAULT_LMMS_EVAL_TIMEOUT_SECONDS
constant near _LMMS_EVAL_PROCESS_CLEANUP_TIMEOUT_SECONDS, set to 3600.0, and use
it as the fallback when both timeout_seconds and timeout are absent in the
timeout resolution logic. Replace the hardcoded 3600 downstream-evaluation
fallback near the existing failure path so both _run_lmms_eval_process behavior
and failure handling use the same default.
- Around line 1042-1044: Update the TimeoutExpired handler around
_signal_lmms_eval_process_group and the final process.communicate() to use a
finite timeout, preserving the existing fallback handling for missing output and
stderr. If the bounded communicate also times out, ensure the runner does not
hang and continues through the established cleanup/error-result path.
In `@tests/unit/torch/puzzletron/test_post_mip_runner.py`:
- Around line 424-429: Update the communicate_timeouts expectation in the
relevant test to use runner._LMMS_EVAL_PROCESS_CLEANUP_TIMEOUT_SECONDS for the
third call instead of None, matching the bounded cleanup communicate behavior in
the post-MIP runner.
---
Outside diff comments:
In `@puzzletron_setup/v2/wizard.py`:
- Around line 4151-4170: Update the prompt flow around the limit, batch_size,
and timeout assignments to check for BACK immediately after each session.integer
call and return it before showing the next prompt, matching
_serving_setting_prompt behavior. Remove the deferred combined check after all
three prompts.
---
Duplicate comments:
In `@puzzletron_setup/wizard.py`:
- Around line 750-753: Update the lmms-eval tasks prompt in the wizard flow to
render sequence defaults as comma-separated task names rather than Python list
syntax, matching _downstream_evaluation_setting_prompt. Add validation that
rejects empty input, ensuring the resulting tasks value cannot become an empty
list before _configured_lmms_eval_tasks processes it.
---
Nitpick comments:
In `@modelopt/torch/puzzletron/post_mip/runner.py`:
- Around line 562-570: Update the _LMMS_EVAL_RESERVED_EXTRA_ARG_FLAGS set to
include --model and --batch_size alongside the existing reserved flags,
preventing extra_args from overriding the runner-emitted backend and batch size
arguments.
In `@tests/unit/torch/puzzletron/test_post_mip_runner.py`:
- Around line 230-278: Refactor the reserved-argument rejection tests around
_lmms_eval_command to use pytest.mark.parametrize for individual cases and
pytest.raises(ValueError) for the expected failures, asserting the error message
within each case. Apply the same pattern to the repeated rejection tests around
lines 518, 565, and 618, preserving each case’s expected message fragments.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 10547945-e8ea-4b90-b1a9-cd73eaf798e7
📒 Files selected for processing (19)
examples/puzzletron/README.mdexamples/puzzletron/ci_environment.jsonexamples/puzzletron/configs/families/nemotron3/nano_30b_a3b_bf16/runs/lmms_eval.yamlexamples/puzzletron/docs/post_mip_pipeline.mdexamples/puzzletron/requirements.txtmodelopt/torch/puzzletron/orchestration/adapters/post_mip.pymodelopt/torch/puzzletron/orchestration/compiler.pymodelopt/torch/puzzletron/orchestration/controller.pymodelopt/torch/puzzletron/orchestration/progress.pymodelopt/torch/puzzletron/post_mip/builtin.pymodelopt/torch/puzzletron/post_mip/reporting.pymodelopt/torch/puzzletron/post_mip/runner.pynoxfile.pypuzzletron_setup/bundle.pypuzzletron_setup/v2/validation.pypuzzletron_setup/v2/wizard.pypuzzletron_setup/wizard.pytests/unit/torch/puzzletron/test_post_mip_runner.pytests/unit/torch/puzzletron/test_setup_bundle.py
💤 Files with no reviewable changes (5)
- modelopt/torch/puzzletron/orchestration/progress.py
- modelopt/torch/puzzletron/post_mip/builtin.py
- modelopt/torch/puzzletron/orchestration/adapters/post_mip.py
- modelopt/torch/puzzletron/orchestration/controller.py
- puzzletron_setup/bundle.py
🚧 Files skipped from review as they are similar to previous changes (4)
- examples/puzzletron/configs/families/nemotron3/nano_30b_a3b_bf16/runs/lmms_eval.yaml
- puzzletron_setup/v2/validation.py
- tests/unit/torch/puzzletron/test_setup_bundle.py
- modelopt/torch/puzzletron/orchestration/compiler.py
Comment on lines
+136
to
+138
| `downstream_evaluation` shells out to `python -m lmms_eval` from the GPU worker | ||
| environment. Install `examples/puzzletron/requirements.txt` in that environment; | ||
| it pins the evaluator package used by the checked-in example: |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the "shells out" wording.
The runner passes an argv list to subprocess.Popen and never invokes a shell. SECURITY.md requires this runner to stay shell-free. The phrase "shells out" states the opposite and could lead a config author to put shell syntax into command_prefix or extra_args, which is not interpreted.
📝 Proposed wording
-`downstream_evaluation` shells out to `python -m lmms_eval` from the GPU worker
-environment. Install `examples/puzzletron/requirements.txt` in that environment;
-it pins the evaluator package used by the checked-in example:
+`downstream_evaluation` runs `python -m lmms_eval` as a subprocess from the GPU
+worker environment. The runner passes an argument list directly and never uses a
+shell. Install `examples/puzzletron/requirements.txt` in that environment; it
+pins the evaluator package used by the checked-in example:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| `downstream_evaluation` shells out to `python -m lmms_eval` from the GPU worker | |
| environment. Install `examples/puzzletron/requirements.txt` in that environment; | |
| it pins the evaluator package used by the checked-in example: | |
| `downstream_evaluation` runs `python -m lmms_eval` as a subprocess from the GPU | |
| worker environment. The runner passes an argument list directly and never uses a | |
| shell. Install `examples/puzzletron/requirements.txt` in that environment; it | |
| pins the evaluator package used by the checked-in example: |
🤖 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 `@examples/puzzletron/docs/post_mip_pipeline.md` around lines 136 - 138, Update
the downstream_evaluation description to state that it invokes python -m
lmms_eval via subprocess without a shell, rather than saying it “shells out.”
Clarify that command_prefix and extra_args are passed as argv elements and must
not contain shell syntax.
Source: Path instructions
Comment on lines
+708
to
+710
| for key in _LMMS_EVAL_MODEL_ARG_FIELDS: | ||
| if key in settings: | ||
| derived[key] = settings[key] |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The frozenset iteration still makes --model_args nondeterministic.
_LMMS_EVAL_MODEL_ARG_FIELDS is a frozenset of strings. Its iteration order depends on per-process hash randomization, so the key order in derived and therefore in _model_arg_string changes between runs for identical configuration. The argv persisted to command.json at line 1083 changes too, and the docstring at line 773 promises a deterministic command.
🛠️ Proposed fix
- for key in _LMMS_EVAL_MODEL_ARG_FIELDS:
+ for key in sorted(_LMMS_EVAL_MODEL_ARG_FIELDS):
if key in settings:
derived[key] = settings[key]🤖 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 `@modelopt/torch/puzzletron/post_mip/runner.py` around lines 708 - 710, Make
the loop over _LMMS_EVAL_MODEL_ARG_FIELDS deterministic by iterating its keys in
a stable order, such as sorted order, before populating derived. Preserve the
existing filtering and value assignment so _model_arg_string, persisted argv,
and the documented command determinism remain stable for identical
configuration.
Comment on lines
+822
to
+823
| timeout = settings.get("timeout_seconds", settings.get("timeout")) | ||
| return argv, env, (float(timeout) if timeout is not None else None) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
A missing timeout_seconds still disables the subprocess timeout.
If the config sets neither timeout_seconds nor timeout, this returns None. _run_lmms_eval_process at line 1035 then calls communicate(timeout=None) and waits forever. A hung vLLM engine blocks the shard worker and holds the GPU allocation with no watchdog.
The failure path at lines 1279-1281 already assumes 3600 as the downstream-evaluation default. Use one shared constant in both places.
🛠️ Proposed fix
- timeout = settings.get("timeout_seconds", settings.get("timeout"))
- return argv, env, (float(timeout) if timeout is not None else None)
+ timeout = settings.get("timeout_seconds", settings.get("timeout"))
+ if timeout is None:
+ timeout = _DEFAULT_LMMS_EVAL_TIMEOUT_SECONDS
+ return argv, env, float(timeout)Define the constant once near _LMMS_EVAL_PROCESS_CLEANUP_TIMEOUT_SECONDS and reuse it at line 1279:
_DEFAULT_LMMS_EVAL_TIMEOUT_SECONDS = 3600.0🤖 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 `@modelopt/torch/puzzletron/post_mip/runner.py` around lines 822 - 823, Define
a shared _DEFAULT_LMMS_EVAL_TIMEOUT_SECONDS constant near
_LMMS_EVAL_PROCESS_CLEANUP_TIMEOUT_SECONDS, set to 3600.0, and use it as the
fallback when both timeout_seconds and timeout are absent in the timeout
resolution logic. Replace the hardcoded 3600 downstream-evaluation fallback near
the existing failure path so both _run_lmms_eval_process behavior and failure
handling use the same default.
Comment on lines
+1042
to
+1044
| except subprocess.TimeoutExpired: | ||
| _signal_lmms_eval_process_group(process, signal.SIGKILL) | ||
| stdout, stderr = process.communicate() |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Bound the final communicate() after SIGKILL.
Line 1044 calls communicate() with no timeout. SIGKILL terminates the direct child, but any descendant that escaped the process group with setsid still holds the inherited stdout and stderr pipes. communicate() then blocks until those descendants exit, and the shard worker hangs inside the timeout handler. SECURITY.md requires bounded execution for the subprocess runner.
🛡️ Proposed fix
except subprocess.TimeoutExpired:
_signal_lmms_eval_process_group(process, signal.SIGKILL)
- stdout, stderr = process.communicate()
+ try:
+ stdout, stderr = process.communicate(
+ timeout=_LMMS_EVAL_PROCESS_CLEANUP_TIMEOUT_SECONDS
+ )
+ except subprocess.TimeoutExpired:
+ stdout, stderr = None, NoneThe existing fallback at lines 1051-1052 already substitutes error.output and error.stderr when the values are None.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| except subprocess.TimeoutExpired: | |
| _signal_lmms_eval_process_group(process, signal.SIGKILL) | |
| stdout, stderr = process.communicate() | |
| except subprocess.TimeoutExpired: | |
| _signal_lmms_eval_process_group(process, signal.SIGKILL) | |
| try: | |
| stdout, stderr = process.communicate( | |
| timeout=_LMMS_EVAL_PROCESS_CLEANUP_TIMEOUT_SECONDS | |
| ) | |
| except subprocess.TimeoutExpired: | |
| stdout, stderr = None, None |
🤖 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 `@modelopt/torch/puzzletron/post_mip/runner.py` around lines 1042 - 1044,
Update the TimeoutExpired handler around _signal_lmms_eval_process_group and the
final process.communicate() to use a finite timeout, preserving the existing
fallback handling for missing output and stderr. If the bounded communicate also
times out, ensure the runner does not hang and continues through the established
cleanup/error-result path.
Source: Path instructions
Comment on lines
+424
to
+429
| assert process.communicate_timeouts == [ | ||
| 7.0, | ||
| runner._LMMS_EVAL_PROCESS_CLEANUP_TIMEOUT_SECONDS, | ||
| None, | ||
| ] | ||
| assert signals == [(5678, signal.SIGTERM), (5678, signal.SIGKILL)] |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
This assertion couples to the unbounded final communicate().
Line 427 asserts the third communicate call receives None. I proposed bounding that call in modelopt/torch/puzzletron/post_mip/runner.py at lines 1042-1044. If you apply that fix, update this expectation to runner._LMMS_EVAL_PROCESS_CLEANUP_TIMEOUT_SECONDS.
🤖 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/puzzletron/test_post_mip_runner.py` around lines 424 - 429,
Update the communicate_timeouts expectation in the relevant test to use
runner._LMMS_EVAL_PROCESS_CLEANUP_TIMEOUT_SECONDS for the third call instead of
None, matching the bounded cleanup communicate behavior in the post-MIP runner.
Signed-off-by: Grzegorz Karch <gkarch@nvidia.com>
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
1 participant
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?
Type of change: new feature, new example, new tests
Adds a Puzzletron dynamic post-MIP
downstream_evaluationnode that evaluates realized checkpoint artifacts withlmms-evalthrough the vLLM backend. The stage builds a deterministic shell-free
lmms-evalcommand, maps the realized checkpoint into vLLM model arguments, derives distributed topology settings from the node config, captures command metadata/stdout/stderr, parses JSON metrics, and publishes flattened task metrics into the post-MIP ledger and report path.
This also extends the Puzzletron setup and orchestration flow so downstream evaluation can be configured with task names, sample limit, batch size, timeout, and vL
LM topology defaults. The orchestration layer now labels downstream evaluation stages in progress/dashboard output and includes resource allocation handling for th
ose stages.
The PR includes a new opt-in Nemotron-3 Nano 30B A3B BF16 example flow that evaluates the best
runtime-075realized model withifevalandgsm8k.Supporting changes include:
max-num-seqs.Usage
Testing
git diff --check feature/puzzletron_v2...HEADpython -m py_compile modelopt/torch/puzzletron/orchestration/adapters/post_mip.py modelopt/torch/puzzletron/orchestration/compiler.py modelopt/torch/puzzletron/\ orchestration/progress.py modelopt/torch/puzzletron/post_mip/builtin.py modelopt/torch/puzzletron/post_mip/reporting.py modelopt/torch/puzzletron/post_mip/runner.p\ y puzzletron_setup/bundle.py puzzletron_setup/v2/parallel_validation.py puzzletron_setup/v2/post_mip.py puzzletron_setup/v2/validation.py puzzletron_setup/v2/wizar\ d.py puzzletron_setup/wizard.py tests/unit/torch/puzzletron/test_orchestration_compiler.py tests/unit/torch/puzzletron/test_post_mip_runner.py tests/unit/torch/puz\ zletron/test_setup_bundle.py tests/unit/torch/puzzletron/test_setup_v2_post_mip.py tests/unit/torch/puzzletron/test_setup_v2_state_validation.py, setup bundle resources, v2 validation, Slurm Enroot/Pyxis paths, direct launcher distributed environment setup, AIPerf vLLM args, and runtime-stat reuse.
lmms-eval:post.params-75.lmms_evalsucceeded on Slurm job15159279.final_reportcompleted on Slurm job15159510.nano/results/smoke/artifacts/campaign_report/campaign_report.html.pytestwas attempted but did not run in this local host environment becausepytestis not installed.Before your PR is "Ready for review"
Make sure you read and follow Contributor guidelines and your commits are signed (
git commi\ t -s -S).Make sure you read and follow the [Security Best Practices](https://github.com/NVIDIA/Model-Optimizer/blob/main/SECURITY.md#security-coding-practices-for-contribut\
ors) (e.g. avoiding hardcoded
trust_remote_code=True,torch.load(..., weights_only=False),pickle, etc.).CONTRIBUTING.md: N/AAdditional Information
lmms-evaland vLLM must be available in the target execution environment. This PR wires Puzzletron to calllmms-eval; it does not vendorlmms-evalor add itas a ModelOpt package dependency.
Summary by CodeRabbit
New Features
lmms-evaland vLLM.ifevalandgsm8k.Documentation
Bug Fixes