Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add class method for Box class which create a box from Lx,Ly,Lz and angles #1234

Merged
merged 42 commits into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
0d6f975
add from lattice_vectors, and to and from lenghts and angles
DomFijan Feb 27, 2024
0717da1
Add methods to convert box dimensions and angles
DomFijan Feb 27, 2024
ae95e63
Merge branch 'main' into feat_from_to_lattice_vectors
DomFijan Mar 7, 2024
5a388bf
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 7, 2024
58a56f6
add type hints and a class method from box lengths and angles for uni…
DomFijan Mar 20, 2024
746b923
add type hints
DomFijan Mar 20, 2024
2f9adcf
Merge branch 'feat_from_to_lattice_vectors' of https://github.com/glo…
DomFijan Mar 20, 2024
c7ad750
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 20, 2024
c399ff3
fix format
DomFijan Mar 20, 2024
2a99532
more formatting fixes
DomFijan Mar 20, 2024
16d028a
fixes to logic and docs to all new functions
DomFijan Mar 20, 2024
dc74488
add tests
DomFijan Mar 20, 2024
7706c2f
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 20, 2024
8860961
update some docstrings for consistency
DomFijan Mar 20, 2024
12b4a1a
Merge branch 'feat_from_to_lattice_vectors' of https://github.com/glo…
DomFijan Mar 20, 2024
0e78e47
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 20, 2024
da9d7ea
linter fix
DomFijan Mar 20, 2024
aa4354a
added changelog and credits
DomFijan Mar 20, 2024
d536985
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 20, 2024
461485e
Test dimension detection for `from_lattice_vectors`
janbridley Mar 20, 2024
90dd30c
Update freud/box.pyx
DomFijan Mar 21, 2024
8e27b3c
apply requested changes
DomFijan Mar 21, 2024
863ac23
Merge branch 'feat_from_to_lattice_vectors' of https://github.com/glo…
DomFijan Mar 21, 2024
5bd2c08
revert unwanted changes and delete tests
DomFijan Mar 21, 2024
6a9b740
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 21, 2024
1a45db1
fix format
DomFijan Mar 21, 2024
ace7fac
final format fix
DomFijan Mar 21, 2024
6f6932e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 21, 2024
ae96836
fix from_box_lengths_and_angles
DomFijan Mar 21, 2024
3aab7cb
fix changelog and credits
DomFijan Mar 21, 2024
a7c18e2
Update freud/box.pyx
tommy-waltmann Mar 21, 2024
c26e88e
Apply suggestions from code review
tommy-waltmann Mar 21, 2024
ae56eff
Update ChangeLog.md
DomFijan Mar 22, 2024
a3daf1b
update docs to state range of angles allowed
DomFijan Mar 22, 2024
dffb772
update test
tommy-waltmann Mar 22, 2024
a3548a1
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 22, 2024
a3449d8
Set nonzero atol for assert_allclose in test_box_Box.py
janbridley Mar 26, 2024
b32ed14
Merge branch 'main' into feat_from_to_lattice_vectors
janbridley Apr 11, 2024
ca710c3
update from_box_lengths_and_angles docstring to contain restrictions …
DomFijan Apr 11, 2024
ad8d8b3
raise error if box has negative volume
DomFijan Apr 11, 2024
b51d8d7
fix sqrt
DomFijan Apr 11, 2024
d78e08e
fix test to check for imposible boxes
DomFijan Apr 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ and this project adheres to

### Added
* New continuous coordination number compute `freud.order.ContinuousCoordination`.
* New methods for conversion of box lengths and angles to/from `freud.box.Box`.

### Removed
* `freud.order.Translational`.
Expand Down
4 changes: 4 additions & 0 deletions doc/source/reference/credits.rst
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,10 @@ Domagoj Fijan
* Contributed code, design, documentation, and testing for ``freud.locality.FilterSANN`` class.
* Contributed code, design, documentation, and testing for ``freud.locality.FilterRAD`` class.
* Added support for ``gsd.hoomd.Frame`` in ``NeighborQuery.from_system`` calls.
* Added support for ``freud.box.Box`` class methods for construction of boxes from cell
lengths and angles (``freud.box.Box.from_box_lengths_and_angles``), as well as a
method for returning box vector lengths and angles
(``freud.box.Box.to_box_lengths_and_angles``).

Andrew Kerr

Expand Down
58 changes: 58 additions & 0 deletions freud/box.pyx
tommy-waltmann marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,27 @@ cdef class Box:
[0, self.Ly, self.yz * self.Lz],
[0, 0, self.Lz]])

def to_box_lengths_and_angles(self):
r"""Return the box lengths and angles.

Returns:
tuple:
The box vector lengths and angles in radians
:math:`(L_1, L_2, L_3, \alpha, \beta, \gamma)`.
"""
alpha = np.arccos(
(self.xy * self.xz + self.yz)
/ (np.sqrt(1 + self.xy**2) * np.sqrt(1 + self.xz**2 + self.yz**2))
)
beta = np.arccos(self.xz/np.sqrt(1+self.xz**2+self.yz**2))
gamma = np.arccos(self.xy/np.sqrt(1+self.xy**2))
L1 = self.Lx
a2 = [self.Ly*self.xy, self.Ly, 0]
a3 = [self.Lz*self.xz, self.Lz*self.yz, self.Lz]
L2 = np.linalg.norm(a2)
L3 = np.linalg.norm(a3)
return (L1, L2, L3, alpha, beta, gamma)

def __repr__(self):
return ("freud.box.{cls}(Lx={Lx}, Ly={Ly}, Lz={Lz}, "
"xy={xy}, xz={xz}, yz={yz}, "
Expand Down Expand Up @@ -921,6 +942,43 @@ cdef class Box:
"positional argument: L")
return cls(Lx=L, Ly=L, Lz=0, xy=0, xz=0, yz=0, is2D=True)

@classmethod
def from_box_lengths_and_angles(
cls, L1, L2, L3, alpha, beta, gamma, dimensions=None,
):
r"""Construct a box from lengths and angles.

All the angles provided must be between 0 and :math:`\pi`.
janbridley marked this conversation as resolved.
Show resolved Hide resolved

Args:
L1 (float):
The length of the first lattice vector.
L2 (float):
The length of the second lattice vector.
L3 (float):
The length of the third lattice vector.
alpha (float):
The angle between second and third lattice vector in radians.
beta (float):
The angle between first and third lattice vector in radians.
gamma (float):
The angle between the first and second lattice vector in radians.
dimensions (int):
The number of dimensions (Default value = :code:`None`).

Returns:
:class:`freud.box.Box`: The resulting box object.
"""
a1 = np.array([L1, 0, 0])
a2 = np.array([L2 * np.cos(gamma), L2 * np.sin(gamma), 0])
a3x = np.cos(beta)
a3y = (np.cos(alpha) - np.cos(beta) * np.cos(gamma)) / np.sin(gamma)
a3z = np.sqrt(1 - a3x**2 - a3y**2)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This line appears to take the square root of a negative number at times, resulting in NaNs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

some combinations of angles are not possible-there is likely a constraint on their sum or something like that. Is this taken into account?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not in how I wrote the test case, which would explain the failures. The code should throw an error if the user gives inputs which violate the constraints.

a3 = np.array([L3 * a3x, L3 * a3y, L3 * a3z])
if dimensions is None:
dimensions = 2 if L3 == 0 else 3
return cls.from_matrix(np.array([a1, a2, a3]).T, dimensions=dimensions)


cdef BoxFromCPP(const freud._box.Box & cppbox):
b = Box(cppbox.getLx(), cppbox.getLy(), cppbox.getLz(),
Expand Down
24 changes: 24 additions & 0 deletions tests/test_box_Box.py
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,30 @@ def test_from_box(self):
box7 = freud.box.Box.from_matrix(box.to_matrix())
assert np.isclose(box.to_matrix(), box7.to_matrix()).all()

def test_standard_orthogonal_box(self):
box = freud.box.Box.from_box((1, 2, 3, 0, 0, 0))
Lx, Ly, Lz, alpha, beta, gamma = box.to_box_lengths_and_angles()
npt.assert_allclose(
(Lx, Ly, Lz, alpha, beta, gamma), (1, 2, 3, np.pi / 2, np.pi / 2, np.pi / 2)
)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think a great test to add would be to start with 3 random lengths and 3 random angles, then call from_box_lengths_and_angles to get a box, then call to_box_lengths_and_angles on that box and assert that you recover the original random lengths and angles.

def test_to_and_from_box_lengths_and_angles(self):
original_box_lengths_and_angles = (
np.random.uniform(0, 100000),
np.random.uniform(0, 100000),
np.random.uniform(0, 100000),
np.random.uniform(0, np.pi),
np.random.uniform(0, np.pi),
np.random.uniform(0, np.pi),
)
box = freud.box.Box.from_box_lengths_and_angles(
*original_box_lengths_and_angles
)
lengths_and_angles_computed = box.to_box_lengths_and_angles()
np.testing.assert_allclose(
lengths_and_angles_computed, original_box_lengths_and_angles, rtol=1e-6
)

def test_matrix(self):
box = freud.box.Box(2, 2, 2, 1, 0.5, 0.1)
box2 = freud.box.Box.from_matrix(box.to_matrix())
Expand Down
Loading