Add support for the new fp_style "cpx" (Quantum Espresso Car-Parrinello cp.x) #1821
Conversation
note: ".str" is taken into account for virial
variable for input file named normally
for more information, see https://pre-commit.ci
|
Heavily relies on this issue being solved deepmodeling/dpdata#899. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## devel #1821 +/- ##
==========================================
+ Coverage 49.80% 49.81% +0.01%
==========================================
Files 83 83
Lines 14904 14978 +74
==========================================
+ Hits 7423 7462 +39
- Misses 7481 7516 +35 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
otherwise incorrect values are calculated
…devel # Conflicts: # examples/qe-cpx/run_machine.json
There was a problem hiding this comment.
Thank you for contribution. I have two suggestions:
(1) Add some information to documentation: https://github.com/deepmodeling/dpgen/blob/devel/doc/overview/supported-software.md
(2) Add your example to https://github.com/deepmodeling/dpgen/blob/devel/tests/test_check_examples.py
|
Thank you for approving this. Would this doc be fine? @njzjz CPMD (Quantum ESPRESSO){dargs:argument} CPMD (Car-Parrinello Molecular Dynamics) is another integration from Quantum ESPRESSO suite, specifically the To use it with DP-GEN, one must prepare a template for the And did I get it right, that I just need to add |
|
@Constant206462 please commit the changes! |
📝 WalkthroughWalkthroughAdds Quantum ESPRESSO Car–Parrinello (cp.x / "cpx") FP support: new fp_style schema, input rendering, execution routing, and post-processing. Includes QE-CPX example inputs/templates/POSCARs, run JSONs, tests, and a duplicated CPMD/QE-cp.x block in documentation. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant DPGEN as DPGEN Orchestrator
participant ArgInfo as Arg Parser
participant Runner as FP Runner
participant QE as Quantum ESPRESSO (cp.x)
User->>DPGEN: submit job with fp_style="cpx"
DPGEN->>ArgInfo: parse fp_style variant
ArgInfo-->>DPGEN: cpx(fp_params:{input_fn, template_fn})
rect rgb(200,230,255)
note right of DPGEN: Input generation (make_fp_cpx)
DPGEN->>Runner: make_fp_cpx(iter, jdata)
Runner->>Runner: render template (%POSITIONS% / %CELL%)
Runner-->>DPGEN: generated P.in + aux files
end
rect rgb(200,255,200)
note right of DPGEN: Execution
DPGEN->>QE: cp.x -in P.in
QE-->>DPGEN: output files
end
rect rgb(255,240,200)
note right of DPGEN: Post-processing (post_fp_cpx)
DPGEN->>Runner: post_fp_cpx(iter, jdata)
Runner-->>DPGEN: serialized FP results
DPGEN-->>User: FP task completion
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
doc/overview/supported-software.md(1 hunks)dpgen/generator/arginfo.py(2 hunks)dpgen/generator/run.py(5 hunks)examples/qe-cpx/POSCAR_SiO2-phosph(1 hunks)examples/qe-cpx/POSCAR_phosphorene(1 hunks)examples/qe-cpx/qe_template_P.in(1 hunks)examples/qe-cpx/qe_template_SiO2P.in(1 hunks)examples/qe-cpx/run_machine.json(1 hunks)examples/qe-cpx/run_param.json(1 hunks)tests/test_check_examples.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
dpgen/generator/run.py (2)
dpgen/generator/lib/utils.py (1)
make_iter_name(16-17)dpgen/util.py (1)
set_directory(163-186)
🔇 Additional comments (9)
examples/qe-cpx/POSCAR_phosphorene (1)
1-25: LGTM! Well-formatted POSCAR file for phosphorene structure.The POSCAR file follows the standard VASP format correctly with proper scaling factor, lattice vectors, atom types, counts, and Cartesian coordinates. The phosphorene structure data appears complete and properly formatted for use with the new CPX workflow.
examples/qe-cpx/POSCAR_SiO2-phosph (1)
1-78: LGTM! Complete POSCAR structure for SiO2-phosph system.The POSCAR file correctly defines a 70-atom SiO2 system with proper format including title line, scaling factor, lattice vectors, element types (Si, O, P), atom counts (18, 36, 16), and all Cartesian coordinates. This provides good test data for the new CPX workflow with a mixed system.
tests/test_check_examples.py (2)
166-166: LGTM! Correctly adds QE-CPX parameter file to test suite.The addition of the qe-cpx run_param.json test ensures the new CPX configuration parameters are validated during testing.
198-198: LGTM! Correctly adds QE-CPX machine file to test suite.The addition of the qe-cpx run_machine.json test ensures the new CPX machine configuration parameters are validated during testing.
dpgen/generator/run.py (5)
3905-3950: LGTM! Well-implemented CPX input generation function.The
make_fp_cpxfunction correctly:
- Uses dpdata to read POSCAR files
- Generates proper cell parameters and atomic positions
- Replaces template placeholders (%CELL%, %POSITIONS%) with actual data
- Maintains proper precision with scientific notation
- Uses the
set_directorycontext manager safelyThe implementation follows the established patterns in the codebase and handles the template substitution properly.
4023-4024: LGTM! Correctly integrates CPX into make_fp_calculation dispatch.The addition of the "cpx" case to the fp_style dispatch correctly calls the
make_fp_cpxfunction, maintaining consistency with other fp_style implementations.
4332-4345: LGTM! Proper CPX integration in run_fp dispatch.The CPX case correctly:
- Defines appropriate input and output files based on the input_fn parameter
- Uses extensions matching CP.x output files (.cel, .evp, .for, .pos, .str)
- Reuses the QE check function (
_qe_check_fin) which is appropriate since cp.x is part of Quantum ESPRESSO- Follows the established pattern for other fp_style cases
4816-4857: LGTM! Well-implemented CPX post-processing function.The
post_fp_cpxfunction correctly:
- Follows the established pattern of other post_fp functions
- Uses dpdata with the "qe/cp/traj" format for parsing cp.x output
- Handles system indexing and file organization properly
- Outputs data in both deepmd/raw and deepmd/npy formats with proper precision
- Includes proper error handling by checking for output file existence
4923-4924: LGTM! Correctly integrates CPX into post_fp dispatch.The addition of the "cpx" case to the post_fp dispatch correctly calls the
post_fp_cpxfunction, completing the integration of CPX into the FP workflow.
already have that in init_data_sys
📝 WalkthroughWalkthroughAdds a new FP style "cpx" (QE cp.x) to the argument schema and FP workflow. Implements input templating and post-processing for cp.x, integrates branches in make_fp_calculation/run_fp/post_fp, and adds example POSCARs, QE input templates, and run configs under examples/qe-cpx/. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Orchestrator
participant Run as run.py
participant CPX as QE cp.x
participant Store as Data storage
Orchestrator->>Run: make_fp_calculation(iter_index, jdata)
Note over Run: fp_style == "cpx"
Run->>Run: make_fp_cpx(...)\n- Read POSCAR\n- Fill template (%CELL%, %POSITIONS%)\n- Write input_fn.in
Orchestrator->>Run: run_fp(iter_index, jdata, mdata)
Run->>CPX: Execute cp.x -in input_fn.in\n(forward_files/backward_files set)
CPX-->>Run: Outputs (.cel, .evp, .for, .pos, .str, output)
Run-->>Orchestrator: _qe_check_fin(log="output")
Orchestrator->>Run: post_fp(iter_index, jdata)
Run->>Run: post_fp_cpx(...)\n- Parse cp.x traj\n- Build dpdata MultiSystems
Run->>Store: Write data.{ss}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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.
@Constant206462 Can you also add a unittest for this feature in tests/generator/test_make_fp.py? You may refer to the unittest for pwscf as a reference.
|
@Yi-FanLi Yeah, sure. But I will be busy until next weekend, so it's unlikely I would do this in next several days. |
|
Well, I'm not sure if I did everything correct here, but hopefully it's alright. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/generator/test_make_fp.py (1)
767-799: Clean up unused variables.Static analysis identified several unused variables:
- Line 775:
mdatais loaded but never used- Line 779:
n_frameis assigned but never used- Lines 780, 782: Loop variables
iiandjjare not used within their loop bodiesApply this diff to remove unused variables:
with open(param_cpx_file) as fp: jdata = json.load(fp) - with open(machine_file) as fp: - mdata = json.load(fp) md_descript = [] nsys = 2 nmd = 3 - n_frame = 10 - for ii in range(nsys): + for _ in range(nsys): tmp = [] - for jj in range(nmd): + for _ in range(nmd): tmp.append(np.arange(0, 0.29, 0.29 / 10)) md_descript.append(tmp)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
tests/generator/context.py(1 hunks)tests/generator/cpx_pyridine_template.in(1 hunks)tests/generator/param-pyridine-cpx.json(1 hunks)tests/generator/test_make_fp.py(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/generator/test_make_fp.py (2)
tests/generator/context.py (1)
setUpModule(55-56)dpgen/generator/run.py (1)
make_fp(3976-3991)
🪛 Ruff (0.13.3)
tests/generator/test_make_fp.py
593-593: Unused noqa directive (non-enabled: UP031)
Remove unused noqa directive
(RUF100)
600-600: Loop control variable idx not used within loop body
(B007)
775-775: Local variable mdata is assigned to but never used
Remove assignment to unused variable mdata
(F841)
779-779: Local variable n_frame is assigned to but never used
Remove assignment to unused variable n_frame
(F841)
780-780: Loop control variable ii not used within loop body
Rename unused ii to _ii
(B007)
782-782: Loop control variable jj not used within loop body
Rename unused jj to _jj
(B007)
🔇 Additional comments (4)
tests/generator/context.py (1)
30-30: LGTM!The addition of
param_cpx_filefollows the established naming convention and pattern used for other FP style parameter files.tests/generator/test_make_fp.py (2)
28-28: LGTM!The import follows the existing pattern for test configuration files.
108-146: LGTM!The CPX input reference string is well-formed and follows Quantum Espresso CP.x input conventions. It will serve as a proper validation baseline for the CPX test.
tests/generator/cpx_pyridine_template.in (1)
1-45: LGTM!The CPX template is well-formed and follows Quantum Espresso CP.x input conventions. The placeholders
%POSITIONS%and%CELL%correctly mark where dpgen will substitute atomic coordinates and cell parameters. The template configuration (electron dynamics, convergence thresholds, etc.) is appropriate for Car-Parrinello calculations.
for some reason in every pyridine param json the masses are incorrect
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tests/generator/test_make_fp.py (1)
592-604: Address the hardcoded filename.The function correctly validates the CPX input header, and the variable shadowing issue from the previous review has been fixed. However, the hardcoded filename
"C5H5N.in"on line 596 remains unaddressed from the previous review.Consider making the filename configurable by reading it from
fp_params.get("input_fn", "C5H5N.in")or document why it must be hardcoded for CPX tests.Apply this diff to make the filename configurable:
-def _check_cpx_input_head(testCase, idx): +def _check_cpx_input_head(testCase, idx, input_fn="C5H5N.in"): fp_path = os.path.join("iter.%06d" % idx, "02.fp") # noqa: UP031 tasks = glob.glob(os.path.join(fp_path, "task.*")) for ii in tasks: - ifile = os.path.join(ii, "C5H5N.in") + ifile = os.path.join(ii, input_fn) testCase.assertTrue(os.path.isfile(ifile))Then update the test call on line 797 to pass the filename from
jdata["fp_params"]["input_fn"]if desired.
Optional: Remove unused
noqadirective.Line 593 has an unused
noqadirective that can be removed for cleaner code.Apply this diff:
- fp_path = os.path.join("iter.%06d" % idx, "02.fp") # noqa: UP031 + fp_path = os.path.join("iter.%06d" % idx, "02.fp")
🧹 Nitpick comments (1)
tests/generator/test_make_fp.py (1)
767-799: LGTM! Test structure follows established patterns.The test class correctly follows the pattern of other FP tests (e.g.,
TestMakeFPPwscf), creating fake MD data, callingmake_fp, and validating the generated files.
Optional: Consider removing unused variables for cleaner code.
Lines 775, 779, 780, and 782 have variables that are assigned but never used (
mdata,n_frame, loop variablesiiandjj). While these follow the pattern of other tests in this file, removing them would reduce noise from static analysis tools.Apply this diff:
with open(param_cpx_file) as fp: jdata = json.load(fp) - with open(machine_file) as fp: - mdata = json.load(fp) md_descript = [] nsys = 2 nmd = 3 - n_frame = 10 - for ii in range(nsys): + for _ in range(nsys): tmp = [] - for jj in range(nmd): + for _ in range(nmd): tmp.append(np.arange(0, 0.29, 0.29 / 10)) md_descript.append(tmp)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/generator/param-pyridine-cpx.json(1 hunks)tests/generator/test_make_fp.py(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/generator/param-pyridine-cpx.json
🧰 Additional context used
🧬 Code graph analysis (1)
tests/generator/test_make_fp.py (2)
tests/generator/context.py (1)
setUpModule(55-56)dpgen/generator/run.py (1)
make_fp(3976-3991)
🪛 Ruff (0.13.3)
tests/generator/test_make_fp.py
593-593: Unused noqa directive (non-enabled: UP031)
Remove unused noqa directive
(RUF100)
600-600: Loop control variable jj not used within loop body
(B007)
775-775: Local variable mdata is assigned to but never used
Remove assignment to unused variable mdata
(F841)
779-779: Local variable n_frame is assigned to but never used
Remove assignment to unused variable n_frame
(F841)
780-780: Loop control variable ii not used within loop body
Rename unused ii to _ii
(B007)
782-782: Loop control variable jj not used within loop body
Rename unused jj to _jj
(B007)
🔇 Additional comments (2)
tests/generator/test_make_fp.py (2)
108-146: LGTM! CPX reference input data is well-structured.The reference input follows Quantum ESPRESSO CP namelist format correctly. The inconsistent trailing commas between parameters are valid in Fortran namelists (commas are optional separators), though a consistent style would be slightly cleaner.
28-28: LGTM! Import correctly added.The
param_cpx_fileimport is properly placed in alphabetical order with other parameter file imports.
Making it possible to use cp.x to calculate energy and forces for structures returned by model_devi.
The cp.x input file at the 02.fp stage is created based on a provided template. This fp style still uses POSCAR for initial system configurations.
Summary by CodeRabbit
New Features
Documentation
Examples
Tests