Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Aug 23, 2025

The amber/md format previously only supported orthogonal unit cells (90° angles), raising a RuntimeError("Unsupported cells") for any simulation with non-orthogonal cells. This limitation prevented users from processing AMBER trajectories with triclinic, monoclinic, or hexagonal crystal systems.

Changes

  • Added cell_lengths_angles_to_cell() function that converts cell lengths and angles to 3×3 cell vector matrices using standard crystallographic conventions
  • Removed orthogonal-only restriction in read_amber_traj() by replacing the angle check with general cell conversion
  • Maintains full backward compatibility - orthogonal cells produce identical results to the previous implementation

Supported Crystal Systems

The implementation now handles all crystal systems:

  • Cubic/Orthorhombic: α = β = γ = 90°
  • Monoclinic: α = γ = 90°, β ≠ 90°
  • Hexagonal: α = β = 90°, γ = 120°
  • Triclinic: α, β, γ ≠ 90°

Example Usage

import dpdata # Previously would raise RuntimeError for non-orthogonal cells system = dpdata.LabeledSystem("simulation", fmt="amber/md") # Now works with any cell angles, including: # - Protein crystals with monoclinic cells # - Hexagonal ice simulations  # - Triclinic organic crystals

Testing

Added comprehensive test coverage including:

  • Unit tests for all crystal system types
  • Edge case validation (extreme angles, multiple frames)
  • Integration tests verifying RuntimeError elimination
  • Backward compatibility verification

All existing amber/md tests continue to pass, ensuring no breaking changes.

Fixes #869.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: njzjz <9496702+njzjz@users.noreply.github.com>
Copilot AI changed the title [WIP] [Feature Request] support non-orthogonal cells for amber/md Add support for non-orthogonal cells in amber/md format Aug 23, 2025
Copilot AI requested a review from njzjz August 23, 2025 02:50
Copilot finished work on behalf of njzjz August 23, 2025 02:50
@njzjz
Copy link
Member

njzjz commented Aug 23, 2025

pre-commit.ci autofix

@codecov
Copy link

codecov bot commented Aug 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.53%. Comparing base (beee99a) to head (14d6447).

Additional details and impacted files
@@ Coverage Diff @@ ## devel #870 +/- ## ========================================== + Coverage 85.48% 85.53% +0.04%  ========================================== Files 82 82 Lines 7607 7625 +18 ========================================== + Hits 6503 6522 +19  + Misses 1104 1103 -1 

☔ View full report in Codecov by Sentry.
📢 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.
@codspeed-hq
Copy link

codspeed-hq bot commented Aug 23, 2025

CodSpeed WallTime Performance Report

Merging #870 will not alter performance

Comparing copilot/fix-869 (14d6447) with devel (beee99a)

Summary

✅ 2 untouched benchmarks

Co-authored-by: njzjz <9496702+njzjz@users.noreply.github.com>
Copilot AI requested a review from njzjz August 23, 2025 05:21
Copilot finished work on behalf of njzjz August 23, 2025 05:21
Co-authored-by: njzjz <9496702+njzjz@users.noreply.github.com>
Copilot finished work on behalf of njzjz August 23, 2025 05:47
Copilot AI requested a review from njzjz August 23, 2025 05:47
@njzjz
Copy link
Member

njzjz commented Aug 23, 2025

pre-commit.ci autofix

Copy link
Member

@njzjz njzjz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current state looks good to me - cell_lengths_angles_to_cell replaces the original code, and this method has a unit test that almost covers all cases. @wanghan-iapcm please double-check since this PR is LLM-generated.

@njzjz njzjz marked this pull request as ready for review August 23, 2025 06:08
@njzjz njzjz requested a review from wanghan-iapcm August 23, 2025 06:09
@njzjz njzjz changed the title Add support for non-orthogonal cells in amber/md format feat: add support for non-orthogonal cells in amber/md format Aug 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants