Skip to content

Conversation

@clpetix
Copy link
Contributor

@clpetix clpetix commented Jun 2, 2025

Adds a method to create a lammpio.Box object from a box matrix.

Copy link
Contributor

@mphoward mphoward left a comment

Choose a reason for hiding this comment

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

Overall looks good! I have two comments below, and there is a documentation build error that needs fixing. We should merge this branch before #80, as I think you will be able to use this method to implement the change in from_hoomd_gsd?


# Extract tilt factors
xy, xz, yz = arr[0, 1], arr[0, 2], arr[1, 2]
tilt = [xy, xz, yz] if numpy.any([xy, xz, yz]) else None
Copy link
Contributor

Choose a reason for hiding this comment

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

This is one area of LAMMPS that is slightly grey. You may want to have a triclinic box that has zero tilt factors, say in order to allow the box to deform later.

Should we add a default argument like check_orthorhombic=True or force_triclinic=False that will cause this check to occur? And otherwise, accept the tilts as they come from the matrix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! I've implemented a force_triclinic argument to cover this scenario!

@clpetix
Copy link
Contributor Author

clpetix commented Jun 3, 2025

I've made these tweaks and fixed the documentation! This should go in before #80, and then I'll made the edits there!

Copy link
Contributor

@mphoward mphoward left a comment

Choose a reason for hiding this comment

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

Thanks! One small comment, then this can be merged.

@mphoward mphoward merged commit a5ff518 into main Jun 3, 2025
15 checks passed
@mphoward mphoward deleted the feature/box-from-array branch June 3, 2025 20:53
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