Skip to content

Commit f30f9cc

Browse files
authored
species/zmat: import vectors as a module to break import cycle (#879)
## Summary - `arc.species.zmat` → `arc.species.vectors` → `arc.species.converter` → `arc.species.zmat` was a real module-level import cycle, latent since 2023. It only stayed harmless because `arc/species/__init__.py` happened to load `converter` before anything reaching `vectors`. CodeQL flagged both ends: `zmat.py:39` (cycle origin) and seven `converter.py:33-39` "may not be defined" errors on the from-name imports of zmat symbols. - **Commit 1 (`f07d1f21`)**: switches `zmat.py` to module-style `from arc.species import vectors` and prefixes the 17 call sites. This was a *necessary but insufficient* step — module imports tolerate partial initialization mid-cycle, but the structural cycle still existed. - **Commit 2 (`0f5d3ad9`)**: actually breaks the cycle by removing `vectors.py`'s module-level import of `converter`. The only call site (`get_vector`) was using `converter.xyz_to_x_y_z(xyz)` to build three full-length axis tuples just to read two atoms' coordinates — replaced with two direct `xyz['coords'][i]` lookups. Net: 6 lines → 3 lines, O(N) → O(1), and the heavy `check_xyz_dict` validation overhead is gone (caller passes a well-formed dict). - After this PR there is no module-level path from `vectors` back to `zmat`, so all 8 CodeQL alerts clear.
2 parents d32a2e6 + 0f5d3ad commit f30f9cc

2 files changed

Lines changed: 21 additions & 24 deletions

File tree

arc/species/vectors.py

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
from arc.common import logger
1010
from arc.exceptions import VectorsError
1111
from arc.molecule.molecule import Molecule
12-
from arc.species import converter
1312

1413

1514
def get_normal(v1: List[float],
@@ -361,11 +360,9 @@ def get_vector(pivot: int,
361360
Returns: list
362361
A vector pointing from the pivotal atom towards the anchor atom.
363362
"""
364-
x, y, z = converter.xyz_to_x_y_z(xyz)
365-
dx = x[anchor] - x[pivot]
366-
dy = y[anchor] - y[pivot]
367-
dz = z[anchor] - z[pivot]
368-
return [dx, dy, dz]
363+
px, py, pz = xyz['coords'][pivot]
364+
ax, ay, az = xyz['coords'][anchor]
365+
return [ax - px, ay - py, az - pz]
369366

370367

371368
def get_lp_vector(label: str,

arc/species/zmat.py

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
from arc.common import get_logger, is_angle_linear, key_by_val
3737
from arc.exceptions import ZMatError, VectorsError
3838
from arc.molecule.molecule import Molecule
39-
from arc.species.vectors import calculate_param, get_vector_length
39+
from arc.species import vectors
4040

4141
if TYPE_CHECKING:
4242
from arc.molecule.molecule import Atom
@@ -244,7 +244,7 @@ def determine_r_atoms(zmat: Dict[str, Union[dict, tuple]],
244244
atom_list_to_explore1, atom_list_to_explore2 = atom_list_to_explore2, []
245245
if len(top) >= 2:
246246
# Calculate the angle formed with the index_atom.
247-
angle = calculate_param(coords=xyz['coords'], atoms=[atom_index] + top[-2:], index=0)
247+
angle = vectors.calculate_param(coords=xyz['coords'], atoms=[atom_index] + top[-2:], index=0)
248248
if not is_angle_linear(angle, tolerance=TOL_180):
249249
linear = False
250250
if len(top) >= 3:
@@ -519,7 +519,7 @@ def determine_d_atoms(zmat: Dict[str, Union[dict, tuple]],
519519
if len(d_atoms) < 4:
520520
for i in reversed(range(len(xyz['symbols']))):
521521
if i not in d_atoms and i in list(zmat['map'].keys()):
522-
angle = calculate_param(coords=coords, atoms=[zmat['map'][z_index]
522+
angle = vectors.calculate_param(coords=coords, atoms=[zmat['map'][z_index]
523523
for z_index in d_atoms[1:] + [i]])
524524
if not is_angle_linear(angle, tolerance=TOL_180):
525525
d_atoms.append(i)
@@ -555,7 +555,7 @@ def determine_d_atoms_without_connectivity(zmat: dict,
555555
for i in reversed(range(n)):
556556
if i not in d_atoms and i in list(zmat['map'].keys()) and (i >= len(zmat['symbols']) or not is_dummy(zmat, i)):
557557
try:
558-
angle = calculate_param(coords=coords, atoms=[zmat['map'][z_index] for z_index in d_atoms[1:] + [i]])
558+
angle = vectors.calculate_param(coords=coords, atoms=[zmat['map'][z_index] for z_index in d_atoms[1:] + [i]])
559559
except VectorsError:
560560
continue
561561
if not is_angle_linear(angle, tolerance=TOL_180):
@@ -566,7 +566,7 @@ def determine_d_atoms_without_connectivity(zmat: dict,
566566
for i in reversed(range(n)):
567567
if i not in d_atoms and i in list(zmat['map'].keys()):
568568
try:
569-
angle = calculate_param(coords=coords, atoms=[zmat['map'][z_index] for z_index in d_atoms[1:] + [i]])
569+
angle = vectors.calculate_param(coords=coords, atoms=[zmat['map'][z_index] for z_index in d_atoms[1:] + [i]])
570570
except VectorsError:
571571
continue
572572
if not is_angle_linear(angle, tolerance=TOL_180):
@@ -632,7 +632,7 @@ def determine_d_atoms_from_connectivity(zmat: dict,
632632
i = 0
633633
atom_a, atom_b, atom_c = atom, zmat['map'][d_atoms[2]], zmat['map'][d_atoms[1]]
634634
while i < len(list(connectivity.keys())):
635-
angle = calculate_param(coords=coords, atoms=[atom_a, atom_b, atom_c])
635+
angle = vectors.calculate_param(coords=coords, atoms=[atom_a, atom_b, atom_c])
636636
if is_angle_linear(angle, tolerance=TOL_180):
637637
num_of_neighbors = len(list(connectivity[atom_a]))
638638
if num_of_neighbors == 1:
@@ -648,7 +648,7 @@ def determine_d_atoms_from_connectivity(zmat: dict,
648648
a_neighbors = connectivity[atom_a]
649649
atom_e = a_neighbors[0] if a_neighbors[0] != atom_b else a_neighbors[1]
650650
if atom_e in list(zmat['map'].values()):
651-
angle = calculate_param(coords=coords, atoms=[atom_e, atom_b, atom_c])
651+
angle = vectors.calculate_param(coords=coords, atoms=[atom_e, atom_b, atom_c])
652652
if is_angle_linear(angle, tolerance=TOL_180):
653653
# E -- B -- C is linear, change indices and test angle F -- B -- C.
654654
atom_a = atom_e
@@ -660,7 +660,7 @@ def determine_d_atoms_from_connectivity(zmat: dict,
660660
# Atom A is connected to at least one other atom not in this linear chain.
661661
for a_neighbor in connectivity[atom_a]:
662662
if a_neighbor != atom_b:
663-
angle = calculate_param(coords=coords, atoms=[a_neighbor, atom_b, atom_c])
663+
angle = vectors.calculate_param(coords=coords, atoms=[a_neighbor, atom_b, atom_c])
664664
if not is_angle_linear(angle, tolerance=TOL_180) \
665665
and a_neighbor in list(zmat['map'].values()) \
666666
and key_by_val(zmat['map'], a_neighbor) not in d_atoms:
@@ -683,7 +683,7 @@ def determine_d_atoms_from_connectivity(zmat: dict,
683683
if len(d_atoms) == 3 and len(connectivity[atom_index]) > 2 \
684684
and connectivity[atom_index][2] in list(zmat['map'].values()) \
685685
and connectivity[atom_index][2] not in [zmat['map'][d_atom] for d_atom in d_atoms[1:]]:
686-
angle = calculate_param(coords=coords, atoms=[zmat['map'][d_atom] for d_atom in d_atoms[1:]]
686+
angle = vectors.calculate_param(coords=coords, atoms=[zmat['map'][d_atom] for d_atom in d_atoms[1:]]
687687
+ [connectivity[atom_index][2]])
688688
if not is_angle_linear(angle, tolerance=TOL_180) \
689689
and connectivity[atom_index][2] in list(zmat['map'].values()) \
@@ -795,7 +795,7 @@ def _add_nth_atom_to_zmat(zmat: Dict[str, Union[dict, tuple]],
795795
# Calculate the angle, add a dummy atom if needed.
796796
added_dummy = False
797797
if a_atoms is not None and all([not re.match(r'X\d', str(zmat['map'][atom])) for atom in a_atoms[1:]]):
798-
angle = calculate_param(coords=coords, atoms=[atom_index] + [zmat['map'][atom] for atom in a_atoms[1:]])
798+
angle = vectors.calculate_param(coords=coords, atoms=[atom_index] + [zmat['map'][atom] for atom in a_atoms[1:]])
799799
if is_angle_linear(angle, tolerance=TOL_180):
800800
# The angle is too close to 180 (or 0) degrees, add a dummy atom.
801801
zmat, coords, n, r_atoms, a_atoms, specific_last_d_atom = \
@@ -885,12 +885,12 @@ def update_zmat_with_new_atom(zmat: dict,
885885
raise ZMatError(f'zmat atom specifications must not have repetitions, got:\n'
886886
f'r_atoms = {r_atoms}, a_atoms = {a_atoms}, d_atoms ={d_atoms}')
887887
if r_atoms is not None:
888-
zmat['vars'][r_str] = calculate_param(coords=coords, atoms=[zmat['map'][atom] for atom in r_atoms])
888+
zmat['vars'][r_str] = vectors.calculate_param(coords=coords, atoms=[zmat['map'][atom] for atom in r_atoms])
889889
if added_dummy:
890890
zmat['vars'][a_str] = 90.0
891891
# The dihedral angle could be either 0 or 180 degrees, depends on the relative position of atom D and B, C
892892
# d_atoms represent the zmat indices of atoms D, C, X, and B.
893-
bcd_angle = calculate_param(coords=coords, atoms=[zmat['map'][d_atoms[3]], zmat['map'][d_atoms[1]],
893+
bcd_angle = vectors.calculate_param(coords=coords, atoms=[zmat['map'][d_atoms[3]], zmat['map'][d_atoms[1]],
894894
zmat['map'][d_atoms[0]]], index=0)
895895
if 180 - TOL_180 < bcd_angle <= 180:
896896
zmat['vars'][d_str] = 180.0
@@ -901,9 +901,9 @@ def update_zmat_with_new_atom(zmat: dict,
901901
f'Expected a linear sequence when using a dummy atom.')
902902
else:
903903
if a_atoms is not None:
904-
zmat['vars'][a_str] = calculate_param(coords=coords, atoms=[zmat['map'][atom] for atom in a_atoms])
904+
zmat['vars'][a_str] = vectors.calculate_param(coords=coords, atoms=[zmat['map'][atom] for atom in a_atoms])
905905
if d_atoms is not None:
906-
zmat['vars'][d_str] = calculate_param(coords=coords, atoms=[zmat['map'][atom]
906+
zmat['vars'][d_str] = vectors.calculate_param(coords=coords, atoms=[zmat['map'][atom]
907907
for atom in d_atoms])
908908
return zmat
909909

@@ -1101,7 +1101,7 @@ def _add_nth_atom_to_coords(zmat: dict,
11011101
if indices[0] == i][0]
11021102
a_index, b_index, c_index = d_indices[3], d_indices[2], d_indices[1]
11031103
# Atoms B and C aren't necessarily connected in the zmat, calculate from coords.
1104-
bc_length = get_vector_length([coords[c_index][0] - coords[b_index][0],
1104+
bc_length = vectors.get_vector_length([coords[c_index][0] - coords[b_index][0],
11051105
coords[c_index][1] - coords[b_index][1],
11061106
coords[c_index][2] - coords[b_index][2]])
11071107
cd_length = zmat['vars'][zmat['coords'][i][0]]
@@ -1116,7 +1116,7 @@ def _add_nth_atom_to_coords(zmat: dict,
11161116
(coords[c_index][1] - coords[b_index][1]) / bc_length,
11171117
(coords[c_index][2] - coords[b_index][2]) / bc_length]
11181118
n = np.cross(ab, ubc)
1119-
un = n / get_vector_length(n)
1119+
un = n / vectors.get_vector_length(n)
11201120
un_cross_ubc = np.cross(un, ubc)
11211121

11221122
# The transformation matrix:
@@ -1649,7 +1649,7 @@ def is_atom_in_linear_angle(i: int, xyz: Optional[dict], mol: Molecule, tol: flo
16491649
continue # avoid redundant pairs and self-pairs
16501650
if i not in (a, b, c):
16511651
continue
1652-
angle = calculate_param(coords=xyz['coords'], atoms=[a, b, c])
1652+
angle = vectors.calculate_param(coords=xyz['coords'], atoms=[a, b, c])
16531653
if is_angle_linear(angle, tolerance=tol):
16541654
return True
16551655
return False
@@ -2141,7 +2141,7 @@ def purge_references_to_atom_0(zmat: dict) -> dict:
21412141

21422142
def safe_calc_param(atoms):
21432143
try:
2144-
return calculate_param(coords=xyz_coords, atoms=atoms)
2144+
return vectors.calculate_param(coords=xyz_coords, atoms=atoms)
21452145
except (ValueError, IndexError, VectorsError):
21462146
return None
21472147

0 commit comments

Comments
 (0)