Don't emit NaN coordinates for a collinear zmat reference

[calvinp0]

What went wrong

zmat_to_coords() could return a geometry containing NaN without failing.

_add_nth_atom_to_coords() builds the reference frame for atom D from the normal of the A-B-C plane, then normalizes it:

n = np.cross(ab, ubc)
un = n / vectors.get_vector_length(n)

When the three reference atoms A, B and C are exactly collinear, n is the zero vector and un becomes NaN. NumPy reports this only as

arc/species/zmat.py:1151: RuntimeWarning: invalid value encountered in divide
  un = n / vectors.get_vector_length(n)

so the NaN flows into the transformation matrix and out as atom D's Cartesian coordinates. Reproduced exactly:

collinear A-B-C  ->  n = [0. 0. 0.]   |n| = 0.0
un = [nan nan nan]
atom D coords = (nan, nan, nan)

Nothing downstream checks, so geometry parsing, isomorphism checks and the following optimization all proceed on NaN. This surfaced in a production run alongside an unrelated scheduler crash.

The fix

A collinear A-B-C has no reference plane, so the dihedral about it is genuinely undefined. Any vector perpendicular to BC is an equally valid reference, and atom D's position remains fully determined by its distance and angle. So the correct response is a deterministic fallback, not an error and certainly not NaN:

n = np.cross(ab, ubc)
n_length = vectors.get_vector_length(list(n))
un = n / n_length if n_length else np.full(3, np.nan)
if not np.all(np.isfinite(un)):
    logger.warning(f'Atoms {a_index}, {b_index} and {c_index} of the zmat are exactly collinear, ...')
    un = get_perpendicular_unit_vector(ubc)

The new get_perpendicular_unit_vector() crosses the input with the Cartesian axis it is least aligned with. That can never itself be degenerate: a unit vector's smallest component is at most 1/sqrt(3), so the cross product has length at least sqrt(2/3). Picking the axis by magnitude rather than arbitrarily keeps the result reproducible.

It warns and names the three atoms, because ARC inserts dummy atoms specifically to keep linear segments out of a Z-matrix. Reaching this branch means one is missing, and that should be visible rather than silently patched over.

The guard tests finiteness, not a length threshold — deliberately

A first attempt used |n| < 1e-10 and regressed the pre-existing TestZMat::test_zmat_to_coords. Instrumenting showed the branch firing on a real zmat in the suite (atoms 0, 7 and 9 placing atom 10) — a near-collinear reference where the existing code still produces a finite, deterministic direction that the test asserts on.

The symptom named the failure mode precisely: invalid value encountered in divide is NumPy's report for 0/0, whereas x/tiny is reported as divide by zero and yields inf. So the defect is n_length being exactly zero, not merely small. Testing not np.all(np.isfinite(un)) fires exactly where NaN would have been produced and nowhere else, leaving near-collinear behaviour untouched.

Evidence

5 new tests plus 9 subtests in TestCollinearReference:

• test_collinear_reference_does_not_produce_nan — without this change: AssertionError: Atom 3 was placed at (nan, nan, nan)
• test_collinear_reference_honours_distance_and_angle — verifies the requested C-D distance and B-C-D angle are both reproduced across three geometries. This is what makes the fallback correct rather than merely finite.
• test_collinear_reference_is_deterministic — the arbitrary plane is chosen reproducibly
• test_non_collinear_reference_is_unchanged — passes both with and without the fix
• test_get_perpendicular_unit_vector — orthogonality and unit length for six input directions

190 passed across zmat_test, converter_test and species_test. Without the fix only the NaN test fails; test_non_collinear_reference_is_unchanged and the pre-existing test_zmat_to_coords both still pass, confirming the change is surgical.

Related

Thematic sibling of #935 (fix_mapping_reverse_discovery), which fixed the same physical degeneracy — collinear reference atoms — in the mapping engine, where it surfaced as Anchors are colinear. This PR is independent of that one and of stack #942.

[Copilot]

Pull request overview

This PR prevents zmat_to_coords() / _add_nth_atom_to_coords() from silently emitting NaN Cartesian coordinates when the three Z-matrix reference atoms defining a dihedral plane are exactly collinear, by introducing a deterministic fallback reference normal and adding regression tests for the collinear case.

Changes:

• Add get_perpendicular_unit_vector() to deterministically construct a perpendicular unit vector for the collinear-reference fallback.
• Update _add_nth_atom_to_coords() to detect the exact 0/0 -> NaN normalization case and fall back while warning.
• Add TestCollinearReference to validate finiteness, distance/angle preservation, and determinism of the fallback.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
arc/species/zmat.py	Adds a perpendicular-vector helper and uses it to avoid NaN coordinates when A–B–C are exactly collinear.
arc/species/zmat_test.py	Adds unit tests covering collinear reference placement behavior and helper correctness.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.42%. Comparing base (9787770) to head (fc46159).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #944      +/-   ##
==========================================
- Coverage   63.53%   63.42%   -0.11%     
==========================================
  Files         114      114              
  Lines       38325    38334       +9     
  Branches    10030    10030              
==========================================
- Hits        24348    24313      -35     
- Misses      11068    11102      +34     
- Partials     2909     2919      +10     

Flag	Coverage Δ
functionaltests	63.42% <ø> (-0.11%)	⬇️
unittests	63.42% <ø> (-0.11%)	⬇️

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

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

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