feat(lammps): load selected dump frames efficiently#1039
Conversation
Add f_idx-based sparse frame loading for LAMMPS dump trajectories while preserving requested order and duplicate indices. Stop scanning after the final requested frame and validate invalid selections explicitly. Coding-Agent: Codex Codex-Version: codex-cli 0.144.6 Model: gpt-5.6-sol Reasoning-Effort: xhigh
Merging this PR will not alter performance
|
There was a problem hiding this comment.
Pull request overview
This PR extends dpdata’s LAMMPS dump loader to support efficient sparse frame loading via a new f_idx selection option, enabling callers to load only specific non-negative frame indices without parsing and storing the entire trajectory.
Changes:
- Added
f_idxsupport todpdata.formats.lammps.dump.load_file()and wired it through the LAMMPS dump plugin (fmt="lammps/dump"), including validation for empty/negative/out-of-range indices and mutual exclusivity with non-defaultbegin/step. - Implemented streaming frame iteration and early-stop behavior (stop after the largest requested index) while preserving requested order and duplicate indices.
- Updated
split_traj()to split frames based on actualITEM: TIMESTEPboundaries rather than assuming a fixed block length, and added unit tests covering ordering/duplicates, scalar index selection, early stopping on file-like objects, and invalid inputs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
dpdata/formats/lammps/dump.py |
Adds streaming frame iteration, f_idx normalization/validation, sparse-loading logic, and robust frame splitting by timestep markers. |
dpdata/plugins/lammps.py |
Plumbs f_idx through the LAMMPS dump format plugin API and documents the new selection behavior/constraints. |
tests/test_lammps_dump_skipload.py |
Adds test coverage for f_idx semantics (order, duplicates, scalar index), validation, and early-stop behavior on file-like inputs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📝 WalkthroughWalkthroughLAMMPS dump loading now accepts specific frame indices, preserves their order and duplicates, validates selection errors, and stops reading after the final target. The plugin forwards ChangesLAMMPS frame selection
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant LAMMPSDumpFormat
participant load_file
participant split_traj
LAMMPSDumpFormat->>load_file: request f_idx
load_file->>split_traj: iterate dump frame lines
split_traj-->>load_file: frame slices
load_file-->>LAMMPSDumpFormat: selected frames
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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.
🧹 Nitpick comments (1)
tests/test_lammps_dump_skipload.py (1)
40-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood coverage; consider adding a test for the
TypeErrorpath.All validation paths (empty, negative, out-of-range, begin/step conflict) are covered, but the
TypeErrorbranch in_normalize_frame_indices(non-integer element, e.g.f_idx=[1.5]orf_idx="x") has no test.🤖 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/test_lammps_dump_skipload.py` around lines 40 - 97, Extend test_invalid_frame_indices to cover the TypeError branch in _normalize_frame_indices by passing a non-integer frame index, such as f_idx=[1.5] or f_idx="x", and assert that TypeError is raised. Match the expected validation message if one is defined.
🤖 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.
Nitpick comments:
In `@tests/test_lammps_dump_skipload.py`:
- Around line 40-97: Extend test_invalid_frame_indices to cover the TypeError
branch in _normalize_frame_indices by passing a non-integer frame index, such as
f_idx=[1.5] or f_idx="x", and assert that TypeError is raised. Match the
expected validation message if one is defined.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 131e18d8-66e6-4c6a-ba86-c7e006b75047
📒 Files selected for processing (3)
dpdata/formats/lammps/dump.pydpdata/plugins/lammps.pytests/test_lammps_dump_skipload.py
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1039 +/- ##
==========================================
- Coverage 87.63% 87.62% -0.01%
==========================================
Files 90 90
Lines 9209 9247 +38
==========================================
+ Hits 8070 8103 +33
- Misses 1139 1144 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Summary
f_idxsupport tolammps/dumpso callers can load arbitrary non-negative frame indices directlySystem.sub_systembegin/stepselectionsITEM: TIMESTEPboundaries instead of assuming a fixed block lengthExample:
Performance benchmark
The benchmark compares direct sparse loading against the existing workflow of loading the complete trajectory and then calling
sub_system(indices).System(..., f_idx=indices)System(...).sub_system(indices)Direct sparse loading was 14.38x faster and reduced peak RSS by 64.39% for this workload. Both methods returned the same 10 selected frames. Because the final frame was selected, both methods traversed the full file; the improvement comes from avoiding storage and parsing of unselected frames.
Validation
python -m unittest test_lammps_dump_skipload.py test_lammps_dump_to_system.py test_lammps_dump_unfold.py test_lammps_dump_shift_origin.py test_lammps_dump_idx.py test_lammps_read_from_trajs.py test_lammps_spin.py— 70 tests passedruff check dpdata/ tests/test_lammps_dump_skipload.py— passedruff format --check dpdata/ tests/test_lammps_dump_skipload.py— passeddpdata --helpanddpdata --version— passedparmeddependency is not installed in the environment, and 43 tests were skippedCloses #367.
Coding agent: Codex
Codex version: codex-cli 0.144.6
Model: gpt-5.6-sol
Reasoning effort: xhigh
Summary by CodeRabbit