fix: skip empty frame properties when dumping deepmd format (#977)#982
Conversation
When an ABACUS SCF run converges but computes no forces/stress (e.g. cal_force/cal_stress disabled, as happens with some GPU/cusolver runs), dpdata produces a LabeledSystem with size-0 forces. Dumping it to deepmd/npy or deepmd/raw wrote a meaningless (nframes, 0) array, which then failed to reload with 'cannot reshape array of size 0 into shape (nframes, natoms, 3)'. Skip empty optional frame properties on dump instead. A missing force/virial file is already a supported state on load, so the round-trip is now consistent. Fixes deepmodeling#977
Merging this PR will not alter performance
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #982 +/- ##
=======================================
Coverage 86.75% 86.76%
=======================================
Files 89 89
Lines 8093 8097 +4
=======================================
+ Hits 7021 7025 +4
Misses 1072 1072 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
📝 WalkthroughWalkthroughTwo early-continue guards are added to the ChangesEmpty-property skip fix and tests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@dpdata/formats/deepmd/comp.py`:
- Around line 155-160: The file has B028 ruff linting violations due to missing
explicit stacklevel keyword arguments in warnings.warn() calls at two locations
in the file. Fix these violations by adding the stacklevel=2 parameter to both
warnings.warn() calls that are missing it. This ensures that warnings are
properly attributed to the caller's code location rather than the warnings
module itself, which is required for compliance with the coding guidelines.
In `@dpdata/formats/deepmd/raw.py`:
- Around line 139-144: Fix the two ruff linting violations (B028) in the file by
adding the explicit stacklevel=2 keyword argument to both warnings.warn() calls.
Locate the two warnings.warn() calls in the file and add stacklevel=2 as a
keyword argument to each one (for example, warnings.warn(..., stacklevel=2)).
This will satisfy the ruff linting requirement to pass ruff check before
committing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5ab6af88-73fd-461d-8443-b71962a9c2a4
📒 Files selected for processing (3)
dpdata/formats/deepmd/comp.pydpdata/formats/deepmd/raw.pytests/test_abacus_pw_scf.py
|
I reviewed this PR and the change looks correct to me. Why I think this is the right fix:
I also ran the relevant local tests: cd tests && uv run --with pytest pytest -q \
test_abacus_pw_scf.py::TestABACUSLabeledOutputNoFS \
test_deepmd_comp.py test_deepmd_raw.py test_deepmd_mixed.pyResult: No blocking comments from me. Reviewed by OpenClaw 2026.6.8 (model: custom-chat-jinzhezeng-group/gpt-5.5). |
Summary
Fixes #977.
When an ABACUS SCF run converges but does not compute forces/stress (e.g.
cal_force/cal_stressdisabled — which is what users hit with the new GPUcusolverks_solver), dpdata's ABACUS parser correctly produces aLabeledSystemwith size-0 forces (this is an intended state, covered byTestABACUSLabeledOutputNoFS).The bug is in the deepmd dump→load round-trip:
dumpdidnp.reshape(forces, [nframes, -1]), which for empty forces writesforce.npyof shape(nframes, 0).On load,
np.reshape((nframes, 0)-array, [nframes, natoms, 3])raises:This is exactly the traceback reported in #977 (surfacing in dpgen's
post_fp_check_fail).Root cause investigation
ModuleIO::print_forceindependent of the KS solver;cusolverfills the wavefunctions just likegenelpa/scalapack_gvx, and forces are computed from the density matrix afterward. There is no code path that disables force output forcusolver/device gpu.FmtTableforce block correctly when present.TOTAL-FORCEblock, which dpdata represents as empty forces — and the deepmd round-trip then corrupts it.Fix
In both
dpdata/formats/deepmd/comp.py(npy) anddpdata/formats/deepmd/raw.py(raw)dump, skip an optional frame property whose array is empty while the system still has frames, rather than writing a meaningless(nframes, 0)array:A missing
force/virialfile is already a supported state on load (_cond_load_datareturnsNoneand skips it), so the round-trip is now consistent.Tests
Added two tests to
TestABACUSLabeledOutputNoFS(reusing the existingINPUT.ch4-noforcestressfixture):test_noforcestress_deepmd_roundtriptest_noforcestress_deepmd_raw_roundtripBoth fail on
masterwith the reportedValueErrorand pass with this change. abacus + deepmd comp/raw/mixed/empty suites pass (312 tests); no new failures in the full suite.Out of scope (user/dpgen side)
This makes dpdata robust to force-less ABACUS results. Producing usable training labels still requires
cal_force 1/cal_stress 1in the generatedINPUTand addingcusolverto dpgen'sks_solverwhitelist.Summary by CodeRabbit
Bug Fixes
Tests