fix: apply CALYPSO run-step options

[njzjz-bot]

Summary

• pass run_config to the CALYPSO model-deviation step
• preserve run-phase continuation and parallelism settings
• add a construction-level regression test with distinct prep/run ratios

Tests

• PYTHONPATH=tests python -m unittest -v tests.test_prep_run_caly.TestPrepRunCalyConfiguration
• isort --check-only dpgen2/superop/prep_run_calypso.py tests/test_prep_run_caly.py
• git diff --check

Closes #354

Coding agent: Codex
Codex version: codex-cli 0.149.0
Model: gpt-5.6-sol
Reasoning effort: xhigh

[coderabbitai]

Warning

Review limit reached

Next included review available in 59 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ebc7fe60-9217-4f4f-9f98-4a6aaec70ec4

📥 Commits

Reviewing files that changed from the base of the PR and between 6b01f29 and 02198b9.

📒 Files selected for processing (2)

• dpgen2/superop/prep_run_calypso.py
• tests/test_prep_run_caly.py

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.43%. Comparing base (6b01f29) to head (02198b9).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #380   +/-   ##
=======================================
  Coverage   84.43%   84.43%           
=======================================
  Files         104      104           
  Lines        6110     6110           
=======================================
  Hits         5159     5159           
  Misses        951      951           

☔ 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.

Retracted. This review was produced without running the mandated /code-review fan-out (the loop skill's section 2); the substitute process used instead has since been shown to miss findings and, in one case, to state a verified-sounding falsehood. Re-reviewing properly.

[wanghan-iapcm]

The one-line fix is correct and I have no objection to it. Blocking only on the test, which pins one key out of four and so does not establish what #354 asked for.

First, a correction. An earlier review of mine on this PR claimed the branch was 112 commits behind master on base fa4a4cf, reported CONFLICTING, failed to import MockedRunCalyDPOptim, and never ran CI. All of that is false for this PR. mergeable: true, base is 6b01f294, and both build (3.9) and build (3.13) pass with the new test covered by codecov. fa4a4cf is a different PR's base — that paragraph was carried over from unrelated analysis. No rebase is needed. Please disregard it.

The fix

Correct, and the blame chain shows it completes a migration that was left half-done twice:

b6c31a5  #188  2024-02-22  step created fully prep-side. Consistent at the time.
2e17cf9  #208  2024-04-01  executor -> run_executor.        left **prep_config
07df321  #217  2024-04-30  template -> run_template_config AND added Slices()
                           -- the commit that made continue_on_* meaningful here.
                           left **prep_config again

So the step has been labelled # run model devi, keyed run-caly-model-devi, templated with run_template_config and executed on run_executor, while taking its step-level kwargs from prep, for two years. This is not cosmetic: examples/almg/input.json and the three examples/water/input_dpgen*.json all set run_explore_config.continue_on_success_ratio to 0.8 and set no prep_explore_config at all, so those users' value was being discarded.

Why this was invisible, and why it matters for the test

The existing fixtures make the two configs indistinguishable. After the same pops _prep_run_caly performs, normalize_step_dict({}) yields:

prep_config -> {'continue_on_failed': False, 'continue_on_num_success': None,
                'continue_on_success_ratio': None, 'parallelism': None}
run_config  -> identical

Swapping prep_config for run_config was a literal no-op under every pre-existing test. Using distinct values, as you have, is exactly the right instinct — the new test does fail when I revert the fix (AssertionError: 0.1 != 0.9). My objection is only that the distinction is drawn on one key.

Blocking: the test cannot catch a partial regression

Details inline. In short, the two fixtures differ only in continue_on_success_ratio, so the other three step-level keys are equal in both and no assertion can tell which dict they came from. I mutated the implementation to spread **run_config while taking continue_on_failed and parallelism back from prep_config, and the suite stayed green.

Not blocking — follow-up material

While checking whether this defect class appears elsewhere, three real ones turned up. None is in scope for this PR; I mention them because they are the same family and worth their own issue:

1. dpgen2/superop/prep_run_diffcsp.py discards both configs entirely. It deep-copies prep_config/run_config, pops template_config / executor / template_slice_config from them, and then spreads neither into any of its three steps (diffcsp-gen, prep-relax, run-relax). Two of those are sliced fan-outs. So for the whole DiffCSP path parallelism and every continue_on_* are silently dropped. Born in 4967951 (#251) and never fixed. That is strictly worse than #354 — this PR's step used the wrong config; those use none.

2. prep-caly-model-devi, the step immediately above the one you fixed, receives no step config at all, and builds its template from run_template_config while running on prep_executor.

3. A latent crash in the same pop/spread family. prep_config never has template_slice_config popped, and non-sliced prep steps are spread with the sliced-only continuation keys — while dargs happily accepts both. Reproduced:

template_slice_config on a prep step config
  -> TypeError: Step.__init__() got an unexpected keyword argument 'template_slice_config'
continue_on_success_ratio on a prep step config
  -> UnboundLocalError: cannot access local variable 'total'   (dflow step.py:970)

#226 fixed exactly this for caly_evo_step.py with the no_slice_run_config idiom; the prep side of prep_run_{calypso,lmp,fp,dp_train}.py never got it.