Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot 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.

@Copilot 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 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

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.

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

@Copilot Copilot AI requested a review from njzjz August 23, 2025 05:21
Copilot finished work on behalf of njzjz August 23, 2025 05:21
Copilot finished work on behalf of njzjz August 23, 2025 05:47
@Copilot 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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request] support non-orthogonal cells for amber/md

2 participants