Skip to content

ExtXYZ support for non-periodic structures - #33

Merged
Alexsp32 merged 3 commits into
mainfrom
support-non-pbc
Jul 9, 2026
Merged

ExtXYZ support for non-periodic structures#33
Alexsp32 merged 3 commits into
mainfrom
support-non-pbc

Conversation

@Alexsp32

@Alexsp32 Alexsp32 commented May 4, 2026

Copy link
Copy Markdown
Member

It’s kind of an oversight we didn’t already include those tbh.

  • Parse missing cell info as non-periodic structures.
  • Unit tests for loading and saving non-periodic structures.
  • Check if documentation needs updating.

@Alexsp32

Alexsp32 commented Jun 1, 2026

Copy link
Copy Markdown
Member Author

@claude[agent] Please finish the remaining tasks here, and open a PR to make documentation changes in the NQCDynamics.jl repo only if necessary.

- Test single and multiple non-periodic frames
- Test writing with Structure objects
- Test writing with Atoms, R, Cell interface
- All tests verify correct InfiniteCell handling

Agent-Logs-Url: https://github.com/NQCD/NQCBase.jl/sessions/ff7d4e35-a160-47ad-bf58-e6d4c89e79a1

Co-authored-by: Alexsp32 <39826690+Alexsp32@users.noreply.github.com>
@Claude

Claude AI commented Jun 1, 2026

Copy link
Copy Markdown

@claude[agent] Please finish the remaining tasks here, and open a PR to make documentation changes in the NQCDynamics.jl repo only if necessary.

All remaining tasks completed in commit 18b1844:

Unit tests added: Comprehensive test coverage for non-periodic structures including single/multiple frames and both Structure and (Atoms, R, Cell) interfaces. All 742 tests pass.

Documentation review: NQCBase.jl documentation doesn't need updates - the docstrings already use AbstractCell which covers both periodic and non-periodic cases.

NQCDynamics.jl documentation: If NQCDynamics.jl has documentation about ExtXYZ I/O, you may want to add a note that non-periodic structures (molecules, clusters) are now supported via InfiniteCell. However, I cannot access that repository to determine if this is necessary. Let me know if you'd like me to create a documentation PR there.

@htsnowden htsnowden left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All looks good to me, though 90% of the line changes I can't even tell whats changed

@Alexsp32
Alexsp32 merged commit 0e570fd into main Jul 9, 2026
1 check passed
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.

3 participants