Linear Cartesian Salcs and maybe other things by sgoodlett · Pull Request #79 · NASymmetry/MolSym

sgoodlett · 2026-09-18T10:35:21Z

Linear point groups now somewhat handled.
Cartesian SALCs available for linear point groups.
Ih symels now align with Prof. Allen's definitions.
goat_check is much more efficient.

Removed the RotationalPolynomials class and its methods.

Removed unnecessary reversal of the basis list.

Update project_Eckart condition to handle 'both' option.

codecov · 2026-09-18T12:27:45Z

Codecov Report

❌ Patch coverage is 90.98712% with 21 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
molsym/pgdetect/flowchart_helper.py	16.66%	10 Missing ⚠️
molsym/pgdetect/flowchart.py	62.50%	3 Missing ⚠️
molsym/salcs/projection_op.py	88.88%	3 Missing ⚠️
molsym/salcs/salc_tools.py	95.74%	2 Missing ⚠️
molsym/symtext/symtext.py	94.28%	2 Missing ⚠️
molsym/salcs/cartesian_coordinates.py	98.76%	1 Missing ⚠️

Flag	Coverage Δ
unittests	`88.10% <90.98%> (+0.95%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
molsym/salcs/nonstandard_frame.py	`90.47% <100.00%> (+0.60%)`	⬆️
molsym/salcs/polynomial_functions.py	`100.00% <ø> (ø)`
molsym/salcs/salc.py	`92.23% <100.00%> (+0.07%)`	⬆️
molsym/symtext/general_irrep_mats.py	`95.32% <100.00%> (+4.63%)`	⬆️
molsym/symtext/goat.py	`55.40% <100.00%> (-15.43%)`	⬇️
molsym/symtext/point_group.py	`95.94% <100.00%> (+7.21%)`	⬆️
molsym/symtext/symel.py	`76.66% <ø> (ø)`
molsym/salcs/cartesian_coordinates.py	`99.43% <98.76%> (-0.57%)`	⬇️
molsym/salcs/salc_tools.py	`91.09% <95.74%> (+1.35%)`	⬆️
molsym/symtext/symtext.py	`84.23% <94.28%> (+4.44%)`	⬆️
... and 3 more

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

nlk36701 · 2026-09-18T16:19:40Z

-            if isinstance(fxn_set, CartesianCoordinates) and project_Eckart is not None:
-                if project_Eckart == "both":
+            if (isinstance(fxn_set, CartesianCoordinates) or isinstance(fxn_set, LinearCartesian)) and project_Eckart is not None:
+                if project_Eckart is "both":


I would change this back to if project_Eckart == "both":

nlk36701 · 2026-09-18T16:32:55Z

        return proj_dipole 


    def dipole_components_to_irrep(self):


Suggested change

def dipole_components_to_irrep(self):

if self.pg.is_linear:

if self.pg.family == "D":

reason = "irrep_mats is None for D-family linear point groups"

else:

reason = (

"irrep_mats for C-family linear point groups is built from a "

"2-element representative symel set, not a proper per-operation "

"matrix set, and is incompatible with this projection formula"

)

raise NotImplementedError(

f"dipole_components_to_irrep is not implemented for linear point groups "

f"(got {self.pg.str}): {reason}."

)

…le atom handling, linear pg dipole irreps

…_to_negative - dipole_components_to_irrep: use self.pg (Symtext has no .symtext) and return every irrep mapped to a list of (xyz, component) tuples; the dict literals repeated keys, so only the last component survived. - ProjectionOp: drop rotational Eckart conditions for a single atom via project_Eckart (the unused local `rotational` had no effect). - salc_tools: add finite_operations()/linear_axis(); maps_to_negative and generate_symmetric_partner use concrete C2v/D2h operations about the molecular axis for linear groups, whose symels have no matrices. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Kh has no discrete symmetry elements or irrep matrices, so the general projection operator cannot run (symtext.order is 0). An atom's x, y, z displacements are the three components of P and are pure translations: projecting out translations leaves no SALCs, otherwise the SALCs are x, y, z. - projection_op: atom_cartesian_salcs(), used by ProjectionOp for Kh. - nonstandard_frame: Kh passes through, only moving the molecule back to its original frame. - salc: sort_partner_functions() handles an empty SALC list. - test/test_atom_salcs.py. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

get_symmetry_equiv_functions_nonstandard stacks one orbit vector per symmetry element, and Kh (a single atom) has none, so building CartesianCoordinates on a nonstandard Kh symtext raised "need at least one array to concatenate". With no symmetry elements there is no dense rrep to guard against, so the standard sets apply. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

subgroup_symtext stored the rotation from the subgroup's standard frame to the parent's frame. When the parent is itself in its standard frame, that is not the molecule's original frame, so nonstandard_symtext(subgroup) came back rotated. Fold the parent's rotation into reverse_rotate/rotate_to_std unless the parent is already nonstandard (already in the original frame). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Linear symmetry elements are abstract (no matrices, no multiplication table), so subgroup_by_name cannot find a subgroup inside them. Orient the subgroup from the molecule instead: the principal axis is the molecular axis and the secondary axis any perpendicular direction, which is a valid choice because rotation about the axis is itself a symmetry. Cs places its mirror to contain the axis (a sigma_v, present in every linear molecule). Any axial group can be requested (C2v, D2h, C3v, D4h, S4, ...); a subgroup the molecule lacks fails in get_atom_mapping. eckart_conditions now recognizes a linear molecule from its zero moment of inertia as well, so it drops the rotation about the axis when the molecule is described by a finite subgroup. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Finite-difference codes that exploit degeneracy displace along one component of a degenerate SALC and need the same quantity (dipole, gradient) for its partner. For linear groups the partner of a Pi component is that component rotated 90 degrees about the molecular axis, and every atom maps onto itself. The companion to generate_symmetric_partner; psi4's findif driver used to do this inline. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

… is a copy - PointGroup.from_string normalizes case ("c2v", "C2V" -> "C2v"), so callers can pass user-typed names straight through (psi4 stores its point-group symbols in lower case). - subgroup_symtext(<own group>) returns a copy of the Symtext. Re-deriving the group's own axes can choose a different but equivalent orientation, e.g. swapping which mirror is sigma_v(0) in C2v, which relabels B1/B2. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Refactor saxis calculation to use more_planar_axis function for better accuracy.

sgoodlett · 2026-10-08T08:01:45Z

+    a, b = np.unravel_index(np.argmax(np.linalg.norm(seps, axis=2)), seps.shape[:2])
+    return seps[a, b] / np.linalg.norm(seps[a, b])
+
+def _map_atoms(symtext, R, tol):


This function seems unnecessary, rotating MolA to MolB should not permute the atoms

Unless it's just rewriting our "get_atom_map" function, in which case this seems redundant

The linear branch of finite_operations re-implemented atom mapping with a private _map_atoms. Wrap its C2v/D2h operation matrices as Symels and use the existing get_atom_mapping instead, as get_linear_atom_mapping already does for inversion. finite_operations drops its now-unused tol argument; mapping uses the molecule's tolerance like the rest of MolSym. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

subgroup_symtext is expected to re-derive the subgroup's standard frame even when the subgroup is the Symtext's own group: largest_D2h_subgroup's reference atom maps and the standard/nonstandard subgroup agreement test depend on it. Callers that want to keep an existing orientation should not request the group they already have. Case-insensitive point-group names are kept. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

nlk36701

Nice

sgoodlett added 6 commits August 23, 2026 18:13

Saving

26bf010

Linear Cinfv Cartesians

c811ba0

Linear Cartesians working?

14e3380

Delete RotationalPolynomials class from polynomial_functions

92ce420

Removed the RotationalPolynomials class and its methods.

Remove reverse operation on basis list

2498612

Removed unnecessary reversal of the basis list.

Merge branch 'main' into irrepmats

2a13771

sgoodlett requested a review from nlk36701 September 18, 2026 12:23

Modify project_Eckart check in projection_op.py

66fd703

Update project_Eckart condition to handle 'both' option.

nlk36701 reviewed Sep 18, 2026

View reviewed changes

Fix comparison operator for project_Eckart condition

4d13a7e

sgoodlett requested a review from nlk36701 September 21, 2026 09:07

sgoodlett and others added 9 commits September 30, 2026 17:34

Addressing Psi4 test failures: new function for C2v orientation, sing…

8bf7a05

…le atom handling, linear pg dipole irreps

Update saxis calculation method in flowchart.py

241b112

Refactor saxis calculation to use more_planar_axis function for better accuracy.

sgoodlett commented Oct 8, 2026

View reviewed changes

nlk36701 and others added 3 commits October 8, 2026 08:44

Refactor axis calculation for Cn values

38e9805

nlk36701 approved these changes Oct 8, 2026

View reviewed changes

nlk36701 merged commit 178e029 into NASymmetry:main Oct 14, 2026
10 checks passed

Navigation Menu

Repository navigation

Linear Cartesian Salcs and maybe other things - #79

Linear Cartesian Salcs and maybe other things#79
nlk36701 merged 20 commits into
NASymmetry:mainfrom
sgoodlett:irrepmats

sgoodlett commented Sep 18, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Sep 18, 2026 •

edited

Loading

Uh oh!

nlk36701 Sep 18, 2026

Uh oh!

sgoodlett Sep 21, 2026

Uh oh!

nlk36701 Sep 18, 2026 •

edited

Loading

Uh oh!

sgoodlett Oct 8, 2026 •

edited

Loading

Uh oh!

sgoodlett Oct 8, 2026

Uh oh!

nlk36701 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

+   def dipole_components_to_irrep(self):
+       if self.pg.is_linear:
+           if self.pg.family == "D":
+               reason = "irrep_mats is None for D-family linear point groups"
+           else:
+               reason = (
+                   "irrep_mats for C-family linear point groups is built from a "
+                   "2-element representative symel set, not a proper per-operation "
+                   "matrix set, and is incompatible with this projection formula"
+               )
+       raise NotImplementedError(
+           f"dipole_components_to_irrep is not implemented for linear point groups "
+           f"(got {self.pg.str}): {reason}."
+       )

Conversation

sgoodlett commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nlk36701 Sep 18, 2026

Choose a reason for hiding this comment

Uh oh!

sgoodlett Sep 21, 2026

Choose a reason for hiding this comment

Uh oh!

nlk36701 Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sgoodlett Oct 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sgoodlett Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

nlk36701 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sgoodlett commented Sep 18, 2026 •

edited

Loading

codecov Bot commented Sep 18, 2026 •

edited

Loading

nlk36701 Sep 18, 2026 •

edited

Loading

sgoodlett Oct 8, 2026 •

edited

Loading