fix: use prepared CALYPSO model by default

[njzjz]

Summary

• detect the backend-specific model filename prepared for a CALYPSO optimization task
• build the default command with frozen_model.pb or model.ckpt.pt accordingly
• verify the TensorFlow default command in the operator test

Tests

• PYTHONPATH=tests python -m unittest -v tests.op.test_run_caly_dp_optim.TestRunDPOptim.test_00_success
• git diff --check

Closes #352

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

Summary by CodeRabbit

• New Features

  • Added support for running Calypso DP optimization tasks.
  • Automatically prepares inputs, executes optimization, and collects optimization and trajectory outputs.
  • Completed tasks are recognized and skipped without generating duplicate results.

• Bug Fixes

  • Failed optimization runs now report transient errors for retry handling.

• Tests

  • Added coverage for successful runs, failures, output collection, and completed-task handling.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b786d987-abf9-4f5e-b036-fa11f6c890f2

📥 Commits

Reviewing files that changed from the base of the PR and between b05af11 and 6ea87da.

📒 Files selected for processing (2)

• dpgen2/op/run_caly_dp_optim.py
• tests/op/test_run_caly_dp_optim.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

Adds RunCalyDPOptim to prepare and execute Calypso DP optimization. The operation detects supported model files, handles unfinished and finished tasks, collects optimization and trajectory artifacts, returns result paths, and includes tests for success and failure paths.

Changes

Calypso DP optimization

Layer / File(s)	Summary
Operation contract and command setup dpgen2/op/run_caly_dp_optim.py	Defines operation inputs and outputs, resolves task files, detects frozen_model.pb or model.ckpt.pt, and builds the optimization command.
Optimization execution and failure handling dpgen2/op/run_caly_dp_optim.py	Links inputs into the work directory, skips finished tasks, executes unfinished tasks, and raises TransientError on command failure.
Artifact collection and validation dpgen2/op/run_caly_dp_optim.py, tests/op/test_run_caly_dp_optim.py	Collects optimization and trajectory outputs, creates empty result directories for finished tasks, returns artifact paths, and tests success, failure, and completed-task behavior.

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

Merge Risk: ⚪ Minimal · up to 6ea87

The change selects the backend-specific prepared CALYPSO model for the default command; no actionable merge-blocking risk remains at the current head, so it is merge-ready after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant TaskInput
  participant RunCalyDPOptim
  participant Calypso
  participant ResultArtifacts
  TaskInput->>RunCalyDPOptim: provide configuration and task metadata
  RunCalyDPOptim->>RunCalyDPOptim: resolve model and link inputs
  RunCalyDPOptim->>Calypso: execute optimization command
  Calypso-->>RunCalyDPOptim: return status and generated files
  RunCalyDPOptim->>ResultArtifacts: copy optimization and trajectory outputs

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main change: using the prepared CALYPSO model as the default.
Linked Issues check	✅ Passed	The implementation selects the prepared backend-specific model filename and tests the TensorFlow default command, satisfying issue #352.
Out of Scope Changes check	✅ Passed	The changes are limited to the CALYPSO optimization operator and focused tests required to implement and verify issue #352.
Docstring Coverage	✅ Passed	Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.

✨ Finishing Touches 💡 1

⚔️ Resolve merge conflicts 💡

• Resolve merge conflict in branch fix/issue-352-calypso-model-command

---

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

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]

Correct fix for a bug that has broken the default configuration for over two years. Approving on the code. Two things about landing it, and one non-blocking note on the test.

The diff is not what GitHub is showing

This branch forked at fa4a4cf (2024-03-16), 113 commits back, before run_caly_dp_optim.py existed. That is why both files render as status: added, +176/-0 and +172/-0. Diffed against actual master the change is:

dpgen2/op/run_caly_dp_optim.py      +10/-2   the model sniff
tests/op/test_run_caly_dp_optim.py   +3/-0   the assertion

and nothing else. It does not revert #208 or #225 — both are fully present in these blobs.

The landing hazard: git merge-tree reports CONFLICT (add/add) on both files. An add/add conflict resolved with --ours produces a clean-looking merge that silently discards the entire fix. Please rebase onto master so this presents as the ten-line change it actually is; failing that, whoever resolves must take theirs on both files.

Why this was broken, for the record

8f9a880 (#201) had it right — it introduced both the sniff and f"python -u calypso_run_opt.py {model_name}". 2e17cf9 (#208) split the OP and the f prefix landed on the key instead of the template:

command = config.get(f"run_opt_command", "python -u calypso_run_opt.py {model_name}")

so the literal string {model_name} was passed to the script. fa4c0db (#225) then fixed the visible symptom by hardcoding model.ckpt.pt — which was wrong on the day it was written, since the default impl is tensorflow, TF freezes to frozen_model.pb, and prep symlinks that name. So the stock pipeline has been broken continuously since 2024-06-01. This PR restores #201's semantics.

The new assertion is real

I ran master's source against this PR's test file:

Expected: run_command('python -u calypso_run_opt.py frozen_model.pb', shell=True)
  Actual: run_command('python -u calypso_run_opt.py model.ckpt.pt', shell=True)

It genuinely kills the regression. Note for anyone reading the history: the fixture has staged frozen_model.pb since #208 while the code default said model.ckpt.pt from #225 onward — the fixture and the code contradicted each other for two years, and only the missing assertion hid it. That is the untested cell.

See the inline note on the test for a cheap way to also pin the selection logic itself. Not blocking.

[wanghan-iapcm]

This is a real regression test — it fails against master's source with the exact expected/actual pair. But it pins the outcome for the TF path, not the selection logic, because setUp stages only frozen_model.pb, so the model.ckpt.pt branch and the fallback are never reached.

I mutated the implementation two ways and re-ran the suite:

model_name = "frozen_model.pb"                       Ran 3 tests ... OK   <- survives
("model.ckpt.pt", "frozen_model.pb")  [order swap]   Ran 3 tests ... OK   <- survives

An unconditional implementation and a reversed preference order both pass. A second case staging model.ckpt.pt instead would kill both and lock down the branch this PR actually adds:

def test_03_pytorch_model(self, mocked_run):
    # stage model.ckpt.pt instead of frozen_model.pb
    mocked_run.assert_called_once_with(
        "python -u calypso_run_opt.py model.ckpt.pt", shell=True
    )

Not blocking the approval.

[wanghan-iapcm]

The logic is right and the .pb-first order correctly mirrors PrepCalyDPOptim's own preference, so when both names somehow exist the run op picks what prep picked. I checked that run_opt_command is declared optional=True with no default=, and that dargs normalization omits the key entirely — so config.get(..., default) really does fall through and this fix is live rather than dead code.

One latent coupling worth knowing about, not asking you to change it here: input_files is built at line 99 with .resolve(), so path.name is the symlink target's basename rather than the staged link name. Today that is harmless because prep does rglob(model_name) then symlink_to, making the two equal. But if models_dir ever contains a same-named symlink pointing at a differently-named real file, the sniff falls through to model.ckpt.pt while a different file is staged — I reproduced that divergence, and it fails silently. Sniffing the un-resolved Path(task_path).iterdir() names would remove the coupling.

Separately: these two literals now live in both this file and prep_caly_dp_optim.py with no shared constant. If prep ever learns a third name, this falls back to model.ckpt.pt and re-creates #352 rather than failing loudly.