Skip to content

Fix Gaussian 1D scan angles losing direction and gaining a spurious 360 - #1037

Merged
calvinp0 merged 1 commit into
mainfrom
fix_gaussian_scan_wraparound
Sep 5, 2026
Merged

Fix Gaussian 1D scan angles losing direction and gaining a spurious 360#1037
calvinp0 merged 1 commit into
mainfrom
fix_gaussian_scan_wraparound

Conversation

@calvinp0

@calvinp0 calvinp0 commented Sep 1, 2026

Copy link
Copy Markdown
Member

Gaussian 1D scan angles lost their direction on a reversed scan and gained a spurious 360 degrees on a
short one.

The defect

arc/parser/adapters/gaussian.py normalised a scan's dihedrals by subtracting the first angle,
lifting negatives by 360, then adding 360 to the last point if it looked too small:

angle -= angle[0]
angle[angle < 0] += 360.0
if len(angle) > 1 and angle[-1] < 2 * (angle[1] - angle[0]):
    angle[-1] += 360.0

Reversed scans. A scan with a negative step produces decreasing dihedrals, so after the
subtraction they are negative and the second line lifts the whole interior — not just the endpoint.
The H₂O₂ series read backwards came out as [0, 350, 340, …, 20, 10, 360]: not monotone, and the
direction is gone. On a reversed sBuOH scan the final point reached 719.999.

Short scans. With two points, angle[0] is 0 after the subtraction, so the guard reduces to
angle[1] < 2 * angle[1] and holds for every positive angle. A 10 degree step parsed as [0, 370].

Three points are not unconditionally affected — uniform steps give 60 < 60, which is false. But
arc/testing/rotor_scans/TS_errored.out, already in the repository, has raw steps of 30.0001 and
29.9999 and therefore parses to [0, 30, 420] today.

That last case points at the origin: the heuristic is a mis-port of Arkane's, which compares against
the declared step size from the log (arkane/ess/gaussian.py:520). ARC substituted the first
observed step, which makes it sensitive to the printed precision of the dihedral.

What the heuristic was protecting

A forward sweep completing a full turn. Gaussian reports dihedrals in (-180, 180], so the last point
of a 360 degree scan equals the first and reads as approximately 0, which would appear to run
backwards. sBuOH.out (46 points) and H2O2.out (37 points) are exactly this case; both still end
at 360, bit for bit unchanged.

The fix

Per-step phase unwrapping. Each raw consecutive difference is wrapped into ±180 with
arc.common.get_angle_in_180_range(step, round_to=None) and cumulatively summed from zero. This
reads the direction from every raw step before any transformation, rather than inferring it from one
transformed step, and it subsumes direction handling instead of branching on it.

round_to=None is required: the default rounds to two decimal places, and rounding per step
accumulates across a long sweep.

Values are deliberately not folded into [0, 360). A sweep may legitimately exceed 360 where that
keeps it monotone, and monotonicity is the property callers rely on.

Verified identical to the previous behaviour on all eleven well-formed Gaussian 1D fixtures; the
output differs only where the previous answer was wrong.

Consumers

Only one consumer reads the angles: scheduler.py:3283 passes them to
plotter.plot_1d_rotor_scan, whose x-axis is already labelled "Dihedral angle increment".
output.py:654, species.py:3026, species.py:3121 (determine_rotor_symmetry, which walks
energies by index) and trsh.py's scan_quality_check all take element [0] — energies only. No
physical quantity changes. Arkane is unaffected: ARC hands it the raw log and RMG-Py performs its own
fold. parse_1d_scan_energies_from_specific_angle now yields a monotone absolute series for a
reversed scan where it previously yielded a scrambled one.

Reuse

np.unwrap(angle, period=360.0) - angle[0] was considered and measured. It is numerically identical
on all fifteen fixtures, so the two differ only on an exactly-180-degree step — and there it is
backwards for real Gaussian input. Gaussian records such a scan as [118.87, -61.13, 118.87], steps
of -180 and +180; np.unwrap's asymmetric half-period rule gives [0, -180, 0], which is not
monotone, where per-step unwrapping gives [0, -180, -360].

signed_angular_diff was the other candidate and is semantically the closer name, but it has no
round_to passthrough, so it would reintroduce the accumulating rounding.

Tests

arc/parser/ — 44 passed, serially and under -n 6 --dist worksteal. Consumers: 238 passed across
trsh_test, output_test, ase_test, plotter_test and species_test.

Validated by mutation rather than coverage — five mutants, each killed by a test: the original
condition restored, direction handling dropped, values folded into [0, 360), a uniform-step
strawman, and the non-optimised-point deletion moved before the unwrap.

The repository had no genuine negative-step or two-point Gaussian log, so both fixtures are derived
from real per-point blocks of H2O2.out (reversed and truncated respectively, with the ModRedundant
directive changed to match). For H₂O₂ this is faithful: with four atoms, every non-scanned degree of
freedom has a unique minimum at each dihedral, so a genuine reversed scan gives the same energies.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MbGeU8wLpafo3YFzTky2ho

Copilot AI lite review requested due to automatic review settings September 1, 2026 10:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The fix is localized, aligns with the stated defect mechanism, and is backed by focused regression tests plus new fixtures covering the previously failing cases.

Pull request overview

This PR fixes how ARC parses Gaussian relaxed 1D scan dihedral angles so the resulting angle series remains monotonic and preserves the scan direction, eliminating incorrect “lift everything by +360” behavior for reversed and short scans.

Changes:

  • Replace the prior endpoint/negative-lifting heuristic with per-step phase unwrapping using get_angle_in_180_range(..., round_to=None) and cumulative summation.
  • Add regression tests to cover forward full-turn wrapping, reversed scans, two-point scans, short scans, and a non-uniform-step case.
  • Add two Gaussian log fixtures to exercise the reversed-scan and two-point-scan scenarios.
File summaries
File Description
arc/parser/adapters/gaussian.py Switches Gaussian relaxed-scan angle normalization to per-step unwrap + cumulative sum to preserve direction/monotonicity.
arc/parser/parser_test.py Adds targeted tests asserting monotonicity and correct direction/values for Gaussian scan angles.
arc/testing/rotor_scans/H2O2_reverse_scan.out New fixture: negative-step Gaussian scan to validate reverse-direction parsing.
arc/testing/rotor_scans/H2O2_two_point_scan.out New fixture: two-point Gaussian scan to prevent spurious +360 behavior.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.67%. Comparing base (b7aa476) to head (1f1ccb4).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1037   +/-   ##
=======================================
  Coverage   65.67%   65.67%           
=======================================
  Files         121      121           
  Lines       41018    41015    -3     
  Branches    10549    10548    -1     
=======================================
+ Hits        26937    26938    +1     
+ Misses      11066    11064    -2     
+ Partials     3015     3013    -2     
Flag Coverage Δ
functionaltests 65.67% <ø> (+<0.01%) ⬆️
unittests 65.67% <ø> (+<0.01%) ⬆️

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.

@calvinp0
calvinp0 force-pushed the fix_gaussian_scan_wraparound branch from 1fc07b0 to 801b64e Compare September 1, 2026 15:40
The last-point +360 heuristic in the Gaussian parser ran on top of an
angle[angle < 0] += 360 fold that had already destroyed the sign of a
negative-step scan, so a reversed sweep came back neither monotone nor
reversed. The guard itself is not a wrap test but a test of whether the final
value is small relative to the first step, which is unconditionally true for a
two-point scan and true for a three-point scan whenever the first observed step
is marginally larger than the last. TS_errored.out, a real log already in the
repository, returned [0, 30, 420] for raw dihedrals 30 degrees apart.

Angles are now unwrapped by accumulating each raw step wrapped into the
-180 to +180 range, which reads the scan direction from the raw values before
any transformation. A full forward sweep still ends at 360 degrees, a reversed
sweep runs monotonically to -360 degrees, and short scans are left alone.
@calvinp0
calvinp0 force-pushed the fix_gaussian_scan_wraparound branch from 801b64e to 1f1ccb4 Compare September 5, 2026 18:20

@kfir4444 kfir4444 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM! great fix.

@calvinp0
calvinp0 merged commit 4d67ad4 into main Sep 5, 2026
8 checks passed
@calvinp0
calvinp0 deleted the fix_gaussian_scan_wraparound branch September 5, 2026 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants