Skip to content

Commit 5a7aa2a

Browse files
authored
Resolve inertia/COM issues (#35, #36, #37) (#73)
Bundles three small, related inertia/centre-of-mass issues (separate PRs would conflict on `thermo.py`/`test_thermo.py`). - **#37** — rename the two non-snake_case inertia attributes `_eigenvalues_I` → `_inertia_eigenvalues` and `_eigenvalues_I_SI` → `_inertia_eigenvalues_si` (clears pylint C0103; pure rename, no behaviour change; the one test reference updated). - **#36** — validate the principal moments of inertia against ASE's `get_moments_of_inertia` for a linear (CO₂) and nonlinear (H₂O) molecule (masses pinned to the tool's), plus a translation-invariance test proving the COM relocation makes the moments position-independent. - **#35** — document that `System.coord()` returns the *original* input coordinates and that the COM relocation is an internal detail of the thermo pipeline (`Thermo._reloc_coord`). Closes #35, closes #36, closes #37. Full suite: 255 passed / 11 skipped.
1 parent 46c8d93 commit 5a7aa2a

3 files changed

Lines changed: 56 additions & 9 deletions

File tree

ThermoScreening/thermo/system.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -739,12 +739,18 @@ def atomic_masses(self) -> List[float]:
739739

740740
def coord(self) -> np.ndarray:
741741
"""
742-
The atom positions of the system.
742+
The atom positions of the system, as originally provided.
743+
744+
These are the input coordinates; they are not centred on the centre of
745+
mass. The centre-of-mass relocation used for the moment-of-inertia /
746+
rotational analysis is an internal detail of the thermo pipeline
747+
(``Thermo._relocate_to_cm`` stores it in ``Thermo._reloc_coord``) and
748+
does not modify the coordinates returned here.
743749
744750
Returns
745751
-------
746752
np.ndarray
747-
The atom positions of the system.
753+
The (original, non-relocated) atom positions of the system.
748754
"""
749755
return np.array([atom.position for atom in self._atoms])
750756

ThermoScreening/thermo/thermo.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ def _compute_rotational_partition_function(self):
227227

228228
if self._n_rot == 0:
229229
# monatomic: no rotational degrees of freedom
230-
self._eigenvalues_I_SI = np.array([])
230+
self._inertia_eigenvalues_si = np.array([])
231231
self._rotational_temperature = np.array([])
232232
self._rotational_constant = np.array([])
233233
self._rotational_temperature_xyz = np.nan
@@ -238,17 +238,17 @@ def _compute_rotational_partition_function(self):
238238
# rotor, the two equal perpendicular moments for a linear molecule
239239
# (eigenvalues are sorted ascending, so the smallest, ~zero for a linear
240240
# molecule, is dropped)
241-
self._eigenvalues_I_SI = (
242-
self._eigenvalues_I[-self._n_rot:]
241+
self._inertia_eigenvalues_si = (
242+
self._inertia_eigenvalues[-self._n_rot:]
243243
* PhysicalConstants["u"]
244244
* PhysicalConstants["A"] ** 2
245245
)
246246
self._rotational_temperature = (
247247
PhysicalConstants["h"] ** 2 / (8 * np.pi**2 * PhysicalConstants["kB"])
248-
) / self._eigenvalues_I_SI
248+
) / self._inertia_eigenvalues_si
249249

250250
self._rotational_constant = (
251-
(((PhysicalConstants["hbar"] ** 2) / 2) / self._eigenvalues_I_SI)
251+
(((PhysicalConstants["hbar"] ** 2) / 2) / self._inertia_eigenvalues_si)
252252
/ PhysicalConstants["h"]
253253
* PhysicalConstants["HztoGHz"]
254254
)
@@ -322,7 +322,7 @@ def _rotational_contribution(self):
322322
self._compute_inertia_tensor()
323323
# the inertia tensor is symmetric by construction; eigvalsh returns real,
324324
# ascending eigenvalues (eig may emit spurious imaginary parts)
325-
self._eigenvalues_I = np.linalg.eigvalsh(self._inertia_tensor)
325+
self._inertia_eigenvalues = np.linalg.eigvalsh(self._inertia_tensor)
326326
self._n_rot = self._rotational_dof()
327327
self._compute_rotational_partition_function()
328328
self._compute_rotational_entropy()

tests/thermo/test_thermo.py

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,47 @@ def test_total_entropy_matches_ase(symbols, positions, freqs, dof, geometry, sig
180180
assert ts_total == pytest.approx(ase_total, abs=0.05)
181181

182182

183+
@pytest.mark.parametrize(
184+
"symbols,positions,freqs,dof",
185+
[
186+
# nonlinear (H2O) and linear (CO2) -> one ~zero moment for the linear case
187+
(["O", "H", "H"], [[0, 0, 0.12], [0, 0.76, -0.48], [0, -0.76, -0.48]],
188+
[1595.0, 3657.0, 3756.0], 3),
189+
(["C", "O", "O"], [[0, 0, 0], [0, 0, 1.16], [0, 0, -1.16]],
190+
[667.0, 667.0, 1333.0, 2349.0], 4),
191+
],
192+
)
193+
def test_inertia_eigenvalues_match_ase(symbols, positions, freqs, dof):
194+
# the principal moments of inertia (amu*A^2, before the SI conversion) must
195+
# match ASE's get_moments_of_inertia for the same geometry and masses
196+
thermo = _ts_thermo(symbols, positions, freqs, dof)
197+
198+
ase_atoms = Atoms(symbols=symbols, positions=positions)
199+
ase_atoms.set_masses([Atom(symbol=s, position=np.zeros(3)).mass for s in symbols])
200+
ase_moments = np.sort(ase_atoms.get_moments_of_inertia())
201+
202+
np.testing.assert_allclose(
203+
np.sort(thermo._inertia_eigenvalues), ase_moments, atol=1e-6
204+
)
205+
206+
207+
def test_inertia_eigenvalues_are_translation_invariant():
208+
# relocating to the centre of mass must make the moments independent of where
209+
# the molecule sits in space
210+
symbols = ["O", "H", "H"]
211+
positions = np.array([[0, 0, 0.12], [0, 0.76, -0.48], [0, -0.76, -0.48]])
212+
freqs, dof = [1595.0, 3657.0, 3756.0], 3
213+
214+
centred = _ts_thermo(symbols, positions, freqs, dof)
215+
shifted = _ts_thermo(symbols, positions + np.array([10.0, -5.0, 3.0]), freqs, dof)
216+
217+
np.testing.assert_allclose(
218+
np.sort(centred._inertia_eigenvalues),
219+
np.sort(shifted._inertia_eigenvalues),
220+
atol=1e-8,
221+
)
222+
223+
183224
from ase.build import molecule # noqa: E402
184225

185226

@@ -465,7 +506,7 @@ def test_thermo_aq_2(self):
465506
np.testing.assert_allclose(thermo._inertia_tensor, np.array([[477.579476, -0.002293, 0.023808], [-0.002293, 1612.635725, 0.000317],[0.023808, 0.000317, 1135.056249]]), atol=1e-4)
466507

467508
# somewhere in the array the values should be the same
468-
np.testing.assert_allclose(np.sort(thermo._eigenvalues_I), np.sort(
509+
np.testing.assert_allclose(np.sort(thermo._inertia_eigenvalues), np.sort(
469510
np.array([1612.635724886585, 477.579474789699, 1135.056250097402])), atol=1e-10)
470511

471512
np.testing.assert_allclose(np.sort(thermo._rotational_temperature), np.sort(

0 commit comments

Comments
 (0)