fix: skip zero-force fabrication when ABACUS MD dump lacks FORCE columns by abhi-0203 · Pull Request #1008 · deepmodeling/dpdata

abhi-0203 · 2026-07-05T04:36:18Z

Summary

When an ABACUS MD dump file does not contain FORCE columns (i.e., ), the parser previously initialized forces as a zero array and always assigned it to the output data dict. This fabricated misleading zero forces instead of correctly omitting the forces key.

Now returns for forces when is , and only includes in the data dict when actual force data was parsed.

Changes

****: Return for forces when is False; guard and data assignment against
****: Add verifying that dumps without FORCE columns do not fabricate zero forces
****: New test fixture with MD dump lacking FORCE columns

Test Plan

All existing tests pass ( — 9/9)
New test passes
Verified that when FORCE is absent from the dump, is not in

Closes #1000

Summary by CodeRabbit

Bug Fixes
- Improved handling of ABACUS MD outputs when force data is missing, preventing errors and ensuring frames are still parsed correctly.
- Unconverged-frame handling now works safely even when no force values are present.
Tests
- Added coverage for MD parsing without force columns.
- Included a new sample input and output dataset to verify coordinates, energies, and cells are still read as expected.

When an ABACUS MD dump file does not contain FORCE columns (i.e., dump_force=0), the parser previously initialized forces as a zero array and always assigned it to the output data dict. This fabricated misleading zero forces instead of correctly omitting the forces key. Now get_coords_from_dump() returns None for forces when calc_force is False, and get_frame() only includes 'forces' in the data dict when actual force data was parsed. Closes deepmodeling#1000

for more information, see https://pre-commit.ci

coderabbitai · 2026-07-05T04:37:23Z

📝 Walkthrough

Walkthrough

The ABACUS MD dump parser now treats forces as optional, allocating None instead of a zero array when FORCE output is absent, and guarding force parsing, frame filtering, and output assignment on this state. A test fixture and unit test validate the no-force scenario.

Changes

ABACUS MD no-force handling

Layer / File(s)	Summary
Conditional force allocation and output `dpdata/formats/abacus/md.py`	`forces` is set to `None` when `calc_force` is false instead of allocating zeros; force population, unconverged-frame deletion, and `data["forces"]` assignment are all guarded on `force is not None`.
No-force test fixture and unit test `tests/abacus.md.noforce/INPUT`, `tests/abacus.md.noforce/OUT.autotest/MD_dump`, `tests/abacus.md.noforce/STRU`, `tests/test_abacus_md.py`	Adds a new ABACUS test fixture without FORCE columns and a `test_no_force_columns` test verifying `forces` is absent while `coords`, `energies`, and `cells` remain present.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

deepmodeling/dpdata#982: Both PRs address ABACUS/DeepMD outputs lacking force data by changing how missing/empty forces are represented and handled.

Suggested reviewers: njzjz

🚥 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 summarizes the main ABACUS MD no-FORCE fix and matches the changeset.
Linked Issues check	✅ Passed	The parser now omits fabricated zero forces when FORCE columns are absent, matching `#1000`'s required behavior.
Out of Scope Changes check	✅ Passed	All code and test changes support the no-FORCE parsing fix and regression coverage, with no unrelated additions.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

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

codspeed-hq · 2026-07-05T04:38:30Z

Merging this PR will not alter performance

⚠️

Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

✅ 2 untouched benchmarks

_{Comparing abhi-0203:fix/abacus-md-force-labels (19e27fb) with master (5de0229)}

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

tests/abacus.md.noforce/OUT.autotest/MD_dump (1)
1-23: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Add tests/abacus.md.noforce/OUT.autotest/running_md.log

dpdata/formats/abacus/md.py:get_frame still unconditionally opens running_md.log, so this fixture will hit FileNotFoundError before test_no_force_columns reaches the no-force path. Add a 2-frame log with matching energies.
🤖 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/abacus.md.noforce/OUT.autotest/MD_dump` around lines 1 - 23, The abacus
MD fixture is missing the running_md.log file that
dpdata/formats/abacus/md.py:get_frame always opens, so the no-force test cannot
proceed. Add a matching 2-frame running_md.log fixture alongside MD_dump, and
make sure its frame count and energies align with the existing MDSTEP entries so
test_no_force_columns can exercise the no-force path without FileNotFoundError.

🧹 Nitpick comments (1)

dpdata/formats/abacus/md.py (1)
118-124: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Redundant forces is not None check.

Since forces is allocated exactly when calc_force is True (Line 75) and never reassigned to None afterwards, forces is not None is always true whenever calc_force is true. The extra check adds no protection and slightly obscures intent.
♻️ Simplify condition
-                if calc_force and forces is not None:
+                if calc_force:
🤖 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 `@dpdata/formats/abacus/md.py` around lines 118 - 124, Simplify the force
parsing condition in the ABACUS MD reader by removing the redundant `forces is
not None` check. In `dpdata.formats.abacus.md`, the `forces` array is already
created in the `calc_force` path and is not reassigned, so update the condition
around the force assignment in the parsing loop to rely on `calc_force` alone
and keep the intent clear.

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

Outside diff comments:
In `@tests/abacus.md.noforce/OUT.autotest/MD_dump`:
- Around line 1-23: The abacus MD fixture is missing the running_md.log file
that dpdata/formats/abacus/md.py:get_frame always opens, so the no-force test
cannot proceed. Add a matching 2-frame running_md.log fixture alongside MD_dump,
and make sure its frame count and energies align with the existing MDSTEP
entries so test_no_force_columns can exercise the no-force path without
FileNotFoundError.

---

Nitpick comments:
In `@dpdata/formats/abacus/md.py`:
- Around line 118-124: Simplify the force parsing condition in the ABACUS MD
reader by removing the redundant `forces is not None` check. In
`dpdata.formats.abacus.md`, the `forces` array is already created in the
`calc_force` path and is not reassigned, so update the condition around the
force assignment in the parsing loop to rely on `calc_force` alone and keep the
intent clear.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cd5c5adc-e4d6-42ad-87f0-2e6a86ce7e66

📥 Commits

Reviewing files that changed from the base of the PR and between 5de0229 and 1cb4958.

📒 Files selected for processing (5)

dpdata/formats/abacus/md.py
tests/abacus.md.noforce/INPUT
tests/abacus.md.noforce/OUT.autotest/MD_dump
tests/abacus.md.noforce/STRU
tests/test_abacus_md.py

The test fixture at tests/abacus.md.noforce/ was missing running_md.log, which dpdata/formats/abacus/md.py:get_frame unconditionally opens. This caused FileNotFoundError when running test_no_force_columns. Add a minimal 2-frame running_md.log with matching energies to fix the test. Addresses CodeRabbit review comment on PR deepmodeling#1008

abhi-0203 · 2026-07-05T07:00:04Z

Addressed CodeRabbit review comments:

Missing fixture (major): Added with 2-frame energy data matching the MD_dump entries. The test now runs without FileNotFoundError.
Redundant check (nitpick): Kept as-is — while the check is technically redundant when , removing it introduces a Pyright type error () because Pyright can't prove is never None at that point. The defensive check is harmless and keeps the type checker happy.

All 10 ABACUS MD tests pass.

codecov · 2026-07-05T07:27:51Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.88%. Comparing base (5de0229) to head (19e27fb).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1008   +/-   ##
=======================================
  Coverage   86.87%   86.88%           
=======================================
  Files          89       89           
  Lines        8268     8270    +2     
=======================================
+ Hits         7183     7185    +2     
  Misses       1085     1085

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

dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. abacus bug Something isn't working dpdata labels Jul 5, 2026

[pre-commit.ci] auto fixes from pre-commit.com hooks

1cb4958

for more information, see https://pre-commit.ci

njzjz requested a review from pxlxingliang July 5, 2026 04:37

coderabbitai Bot reviewed Jul 5, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: skip zero-force fabrication when ABACUS MD dump lacks FORCE columns#1008

fix: skip zero-force fabrication when ABACUS MD dump lacks FORCE columns#1008
abhi-0203 wants to merge 3 commits into
deepmodeling:masterfrom
abhi-0203:fix/abacus-md-force-labels

abhi-0203 commented Jul 5, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 5, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

codspeed-hq Bot commented Jul 5, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

abhi-0203 commented Jul 5, 2026

Uh oh!

codecov Bot commented Jul 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

abhi-0203 commented Jul 5, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Test Plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

codspeed-hq Bot commented Jul 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

abhi-0203 commented Jul 5, 2026

Uh oh!

codecov Bot commented Jul 5, 2026

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

abhi-0203 commented Jul 5, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 5, 2026 •

edited

Loading

codspeed-hq Bot commented Jul 5, 2026 •

edited

Loading