Repository navigation
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1052 +/- ##
==========================================
- Coverage 66.18% 66.17% -0.02%
==========================================
Files 122 122
Lines 41824 41825 +1
Branches 10751 10751
==========================================
- Hits 27680 27676 -4
- Misses 11084 11093 +9
+ Partials 3060 3056 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The third argument of an ORCA `%geom Scan` line is the number of points the scan visits, not the increment between them. Two fixtures in this repo settle it: `arc/testing/rotor_scans/orca/cc.txt` echoes the input `D 2 1 0 3 = -180.0, 180.0, 45.0` and the run it produced holds 45 RELAXED SURFACE SCAN STEP banners and a 45-row surface table stepping by 360/44 = 8.1818 degrees; `dft.txt` echoes `..., -180.0, 180.0, 40.0` and holds 40. Read as a step size those would have been 9 and 10 points. The adapter wrote `<dihedral>, <dihedral - scan_res>, <scan_res>`, so every ARC-run ORCA rotor scan asked for 8 points across an 8-degree window, swept backwards from the starting geometry, and called it a hindered rotor. The resulting potential covers 2% of the rotation, so the Fourier fit, the barrier height and the symmetry number derived from it were all meaningless. No test asserted the block, which is how it survived since 28bb254. It now writes `<dihedral>, <dihedral + 360>, <360 / scan_res + 1>`: a full rotation from the species' current dihedral, sampled every `scan_res` degrees, whose final point returns to the starting dihedral. That is the same angle grid the Gaussian adapter's `D <scan> S <360 / scan_res> <scan_res>` produces, so the two ESSs now yield comparable scans. The duplicated endpoint is what ARC's downstream already expects: `scan_quality_check` tests `energies[-1]` against `energies[0]` for hysteresis and pairs conformer 0 with conformer N at `delta = 0`, and `determine_rotor_symmetry` indexes cyclically with an explicit branch for a first and last point of equal energy. Stopping one step short would make the hysteresis check compare two genuinely different geometries. The point count is `int(360 / scan_res) + 1` and the span `int(360 / scan_res) * scan_res`, which keeps the step exactly `scan_res` and the count an integer. No new guard is needed for a resolution that does not divide 360: both `arc/main.py::check_rotor_scan_resolution` and `arc/job/adapters/common.py::check_argument_consistency` already reject one before the adapter is built, and the only troubleshooting override, `trsh_scan_job`, yields 4, 2 or 1. The block terminator is fixed alongside it. A `Scan` block and the `%geom` enclosing it each close with an `end`, so the count cannot depend on the number of torsions, yet a single-torsion scan wrote one `end` and left `%geom` open while a multi-torsion scan wrote two. Both fixtures show the two. The docstring now also records that several `D` lines in one block are an ORCA nested surface scan, so N torsions cost `(360 / scan_res + 1) ** N` optimizations rather than describing N separate rotations.
calvinp0
force-pushed
the
fix_orca_scan_point_count
branch
from
September 28, 2026 07:24
ec0a877 to
d811fd9
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The defect
ORCA's third argument in a
%geom Scanconstraint is a number of points, not a step size. ARC treated it as a step size, so every ORCA rotor scan it generated swept a few degrees instead of a full rotation.For a torsion at 92.8° with the default
rotor_scan_resolution = 8.0, ARC asked for:which ORCA reads as 8 points spanning 8 degrees, backwards.
Two fixtures in the repository settle the semantics independently.
arc/testing/rotor_scans/orca/cc.txt:209echoesD 2 1 0 3 = -180.0, 180.0, 45.0and the run contains 45RELAXED SURFACE SCAN STEPbanners with a 45-row surface table stepping by 360/44 = 8.18°. A step size of 45 would have given 9 rows.dft.txt:211echoes a third argument of40.0and produces 40 steps, not 9.A second defect sat on the same lines: a single-torsion scan closed the block with one
endwhile the multi-torsion path wrote two, leaving%geomunterminated. Both fixtures show two. Every ARC-generated ORCA 1D rotor input was therefore syntactically invalid as well as mis-ranged.The change
46 points spanning 360° forwards, step exactly 8.0.
The endpoint deliberately duplicates the start.
arc/job/trsh.py:1729invalidates a scan onabs(energies[-1] - energies[0]) > inconsistency_az, a hysteresis test that is only meaningful when the last point is the same dihedral as the first;trsh.py:1621-1631buildsint(360 / scan_res) + 1conformers on the same convention, anddetermine_rotor_symmetrycarries a wrap-around branch that exists only for a duplicated endpoint. Thecc.txtfixture shows how tight a real closure is: −180° and +180° differ by 0.0022 kJ/mol.The point count is
int(360 / scan_res) + 1, so the step is exactlyscan_resand the count is always an integer. No guard is added for a resolution that does not divide 360, becausearc/main.py:1334andarc/job/adapters/common.py:709already reject one, the latter inside_initialize_adapter, which covers directly-constructed adapters.The sweep is now positive, matching the Gaussian, xtb, TeraChem and Q-Chem adapters. The start value is not folded into ±180:
calculate_dihedral_anglereturns 0–360 and the three sibling adapters passdihedral + 360.0unfolded in production today.Tests
arc/job/adapters/orca_test.pynever asserted the generated%geom Scanblock, which is why this shipped. 12 tests now pin it — the rendered input verbatim, the point count, start, end and implied step, the resolution table (8→46, 4→91, 2→181, 1→361), integer rendering, the multi-torsion case, block closure, the TS case, and a start above 180°. All 12 fail against the previous line.A cross-ESS test renders a real Gaussian scan input and asserts ORCA's point count equals Gaussian's step count plus one, so the two programs stay comparable.
arc/job/adapters/orca_test.py+arc/parser/: 188 passed.Note for reviewers
The companion parser-side change, which converts ORCA scan energies from absolute Hartree to the relative kJ/mol every consumer expects, is in #917 (
arc/parser/adapters/orca.py). Both are needed for an ORCA rotor scan to produce a usable barrier.🤖 Generated with Claude Code
https://claude.ai/code/session_01MndwJm4BEs4DN3vJQsEwD3