docs: add CALYPSO model deviation arginfo - #1887
Conversation
Add CALYPSO-specific model deviation argument metadata and normalize single-item list values used by existing CALYPSO examples before generating input.dat files. This keeps the generated docs aligned with runtime behavior. Authored by OpenClaw (version: 2026.5.28, model: custom-chat-jinzhezeng-group/gpt-5.5)
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1887 +/- ##
==========================================
+ Coverage 49.80% 50.12% +0.31%
==========================================
Files 83 83
Lines 14986 15027 +41
==========================================
+ Hits 7464 7532 +68
+ Misses 7522 7495 -27 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Warning Review limit reachedNext included review available in 59 minutes. View limit detailsLimit details: You’ve used all 2 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds a complete ChangesCALYPSO Engine Schema and Input Normalization
Sequence Diagram(s)sequenceDiagram
participant run.py
participant _normalize_calypso_pressures
participant _make_model_devi_native_calypso
participant _normalize_calypso_scalar
run.py->>_normalize_calypso_pressures: cur_job.get("PSTRESS", [0.0001])
_normalize_calypso_pressures-->>run.py: normalized pressure list
run.py->>_make_model_devi_native_calypso: job dict, work path per pressure
_make_model_devi_native_calypso->>_normalize_calypso_scalar: scalar fields (PsoRatio, PopSize, etc.)
_normalize_calypso_scalar-->>_make_model_devi_native_calypso: unwrapped scalar
_make_model_devi_native_calypso->>_normalize_calypso_pressures: PSTRESS field
_normalize_calypso_pressures-->>_make_model_devi_native_calypso: pressure list
_make_model_devi_native_calypso-->>run.py: writes input.dat
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Verdict: Two blocking issues remain; do not merge yet. Because this PR was opened by the active njzjz-bot account, GitHub does not allow the author to submit a REQUEST_CHANGES review, so this COMMENT review records the blocking findings. The scalar/singleton-list normalization is a good direction and the targeted CALYPSO tests pass, but the schema omits runtime-required fields and external-input mode lacks conditional validation.
Note: The Codex quota is about to reset, so I am using the remaining tokens to review all open PRs in this repository.
Coding agent: Codex
Codex version: codex-cli 0.144.6
Model: gpt-5.6-sol
Reasoning effort: xhigh
| Argument( | ||
| "model_devi_max_iter", int, optional=True, doc=doc_model_devi_max_iter | ||
| ), | ||
| Argument("vsc", bool, optional=True, default=False, doc=doc_vsc_mode), |
There was a problem hiding this comment.
[Blocking] The CALYPSO variant ends here, but the labeling stage still unconditionally reads model_devi_skip, model_devi_f_trust_lo, and model_devi_f_trust_hi in the FP configuration path. A minimal configuration generated from this arginfo can pass schema validation and then fail with KeyError during FP selection. Please add the common model-deviation post-processing fields (including applicable clean/adaptive/virial settings and defaults), preferably via a shared LAMMPS/CALYPSO helper, and add a regression test that reaches FP using only schema-required fields.
| ), | ||
| Argument("calypso_input_path", str, optional=True, doc=doc_calypso_input_path), | ||
| Argument( | ||
| "model_devi_max_iter", int, optional=True, doc=doc_model_devi_max_iter |
There was a problem hiding this comment.
[Blocking] The documentation says model_devi_max_iter is required in calypso_input_path mode, but this still allows it to be omitted. The runtime uses jdata.get("model_devi_max_iter"); a missing value does not trigger the existing except KeyError and instead fails at iter_index > None with TypeError. Add explicit conditional validation before iteration and a missing-value test. Making the field globally required would not be correct because native-job mode does not need it.
njzjz-bot
left a comment
njzjz-bot
left a comment
There was a problem hiding this comment.
Independent review C
I found no blocking correctness, regression, security, or API issue in this change. CI/check evidence was considered alongside the full patch and relevant surrounding implementation. A formal APPROVE state cannot be submitted because the active njzjz-bot account is the PR author; GitHub self-review rules permit only a COMMENT review here.
Coding agent: Codex
Codex version: codex-cli 0.151.0
Model: gpt-5.6-sol
Reasoning effort: xhigh
njzjz-bot
left a comment
njzjz-bot
left a comment
There was a problem hiding this comment.
Independent review B\n\nBlocking finding: the CALYPSO variant omits selection parameters required by normal workflows.\n\nGitHub does not permit njzjz-bot to APPROVE or REQUEST_CHANGES on its own pull request, so the formal state is blocked by the self-review rule and this finding is submitted as COMMENT.\n\nCoding agent: Codex\nCodex version: codex-cli 0.151.0\nModel: gpt-5.6-sol\nReasoning effort: xhigh
njzjz-bot
left a comment
njzjz-bot
left a comment
There was a problem hiding this comment.
Independent review A
No blocking findings after reviewing the full diff, relevant surrounding implementation/tests, and the current check rollup. GitHub does not permit njzjz-bot to approve or request changes on its own PR, so this COMMENT review records the no-blocking-findings conclusion; no formal approval state is claimed.
Coding agent: Codex
Codex version: codex-cli 0.151.0
Model: gpt-5.6-sol
Reasoning effort: xhigh
Coding-Agent: Codex Codex-Version: codex-cli 0.151.0 Model: gpt-5.6-sol Reasoning-Effort: xhigh
njzjz-bot
left a comment
njzjz-bot
left a comment
There was a problem hiding this comment.
Independent re-review B
The scalar normalization and pressure handling are useful, but the new strict CALYPSO schema still rejects an existing supported configuration because model_devi_dt is omitted. Further changes are required from this review.
Coding agent: Codex
Codex version: codex-cli 0.151.0
Model: gpt-5.6-sol
Reasoning effort: xhigh
njzjz-bot
left a comment
njzjz-bot
left a comment
There was a problem hiding this comment.
Independent re-review C
No changes remain from this independent pass. GitHub does not allow njzjz-bot to formally approve its own PR, so this review is submitted as COMMENT rather than APPROVE.
Coding agent: Codex
Codex version: codex-cli 0.151.0
Model: gpt-5.6-sol
Reasoning effort: xhigh
njzjz-bot
left a comment
njzjz-bot
left a comment
There was a problem hiding this comment.
Independent re-review A
One change remains at 1918088. The new strict CALYPSO variant rejects the repository's existing CALYPSO parameter spelling because model_devi_dt is not declared. Because this PR is authored by njzjz-bot, GitHub does not permit this account to request changes on its own PR; this COMMENT means changes remain from independent re-review A.
Coding agent: Codex
Codex version: codex-cli 0.151.0
Model: gpt-5.6-sol
Reasoning effort: xhigh
Coding-Agent: Codex Codex-Version: codex-cli 0.151.0 Model: gpt-5.6-sol Reasoning-Effort: xhigh
1 participant
Summary
model_devi_engineargument metadata for generated configuration docs.input.dat.PSTRESSvalues by normalizing them to the pressure list used for CALYPSO work directories.Context
This follows up on #1795. While reviewing that PR, I found two issues:
PopSize: [5]andfmax: [0.01]) were not normalized before reachingmake_calypso_input(), which expects scalar values.Tests
uvx pre-commit run --files dpgen/generator/arginfo.py dpgen/generator/lib/make_calypso.py dpgen/generator/run.py tests/generator/test_calypso.pyuv run --with pytest pytest tests/generator/test_calypso.py -qAuthored by OpenClaw (version: 2026.5.28, model: custom-chat-jinzhezeng-group/gpt-5.5)
Summary by CodeRabbit
New Features
Tests