[alongd]

• New module arc/species/nd_scan.py: Extracts ND scan logic from scheduler.py into a dedicated subsystem with clean interfaces for grid generation, continuous scan state, adaptive sparse 2D sampling, surface validation, coupling metrics, and rotor classification
• Adaptive sparse 2D scans: For eligible brute_force_sp/brute_force_opt 2D rotors, ARC can now run batched adaptive sampling instead of the full Cartesian grid, controlled by a sampling_policy field on the rotor — no new directed_scan_type values are introduced
• Surface validation: Neighbor-edge continuity checks, periodic boundary consistency, and branch-jump detection diagnose PES discontinuities without blocking finalization
• Coupling analysis and classification: Separable-fit residual analysis classifies each adaptive 2D rotor as separable, coupled, or unreliable with an explicit recommended action
• Sparse-aware 2D plotting: plot_2d_rotor_scan dispatches to an interpolation-based sparse contour path for adaptive scans, with sampled/failed/invalid point overlays
• xTB directed_scan fix: The xTB adapter now correctly runs --opt for brute_force_opt directed scan jobs (was silently running SP)
• Bug fix: Corrected parse_e_elect(path=...) → parse_e_elect(log_file_path=...) kwarg in check_directed_scan_job

[Copilot]

Pull request overview

Refactors ARC’s N-dimensional (ND) directed-rotor scan logic into a dedicated arc/species/nd_scan.py subsystem, adds an adaptive sparse 2D sampling path with surface validation + coupling classification, and updates the scheduler/plotter/xTB adapter to support the new flow.

Changes:

• Extracted ND scan utilities from scheduler.py into arc/species/nd_scan.py and updated scheduler call sites accordingly.
• Added adaptive sparse 2D scan execution (batch selection, bookkeeping, surface validation, coupling metrics, and rotor classification) with new unit/functional tests.
• Updated 2D plotting to support sparse/adaptive scans via interpolation-based contours with overlays; fixed xTB directed-scan opt behavior.

Reviewed changes

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

Show a summary per file

File	Description
arc/species/nd_scan.py	New ND scan subsystem incl. adaptive sparse 2D sampling, validation, coupling metrics, and classification.
arc/species/nd_scan_test.py	Unit tests covering dense/continuous helpers and the new adaptive/validation/classification logic.
arc/scheduler.py	Integrates nd_scan helpers; adds adaptive batch spawning and refactors directed-scan finalization/recording.
arc/scheduler_test.py	Functional xTB-backed tests for dense + adaptive 2D directed scans.
arc/plotter.py	Adds sparse/adaptive 2D plotting path (interpolation + overlays) and dispatch logic.
arc/plotter_test.py	Tests for sparse plotting helpers and interpolation behavior.
arc/job/adapters/xtb_adapter.py	Ensures --opt is used for directed_scan jobs whose directed-scan type implies optimization.

---

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

[Copilot]

Pull request overview

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

---

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

[Copilot]

Pull request overview

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

---

💡 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 67.70%. Comparing base (828e7a6) to head (f8420dd).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #855      +/-   ##
==========================================
+ Coverage   67.19%   67.70%   +0.50%     
==========================================
  Files         123      124       +1     
  Lines       43674    44511     +837     
  Branches    11153    11307     +154     
==========================================
+ Hits        29348    30135     +787     
+ Misses      11189    11149      -40     
- Partials     3137     3227      +90     

Flag	Coverage Δ
functionaltests	67.70% <ø> (+0.50%)	⬆️
unittests	67.70% <ø> (+0.50%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 15 files with indirect coverage changes

🚀 New features to boost your workflow:

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

[Copilot]

Pull request overview

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

---

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

[Copilot]

Pull request overview

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

---

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

[alongd]

I rebased this branch onto current main (263 commits of drift) as nd_scans_rebased, pushed it to the canonical repo, and opened #930 from it — deliberately not touching nd_scans here, since that branch is checked out in a live working session with uncommitted local patches and shouldn't be force-pushed or rebased in place. Below are the review findings from doing that rebase and a follow-up read-through.

Conflict resolution note (for the author)

At arc/scheduler.py, in check_directed_scan_job, neither side of the merge conflict was correct wholesale, so I kept both:

• Current main has a TS isomorphism short-circuit (if is_ts: is_isomorphic = True) that this branch had dropped. Without it, the new invalid-point routing would send every point of a TS directed scan to mark_scan_point_invalid and discard the surface.
• This branch has the correct parse_e_elect(log_file_path=...) kwarg that main lacks (see bug Create README.md #1 below).

The resolution in nd_scans_rebased keeps both the TS guard and the corrected kwarg.

Two latent bugs on main itself (not introduced by this PR)

1. arc/scheduler.py calls parser.parse_e_elect(path=...), but make_parser (arc/parser/parser.py) builds a callable whose parameter is log_file_path — path is only the {path} key in the error-message template. Any completed directed-scan job raises TypeError on main today. This is masked in the test suite by a MagicMock standing in for the parser. This branch already has the correct kwarg, which is why it surfaced as a merge conflict rather than a silent bug.
2. Scheduler.__init__ calls run_scan_jobs() from its per-species restart loop before self.pipe_planner is assigned, and run_scan_jobs dereferences it via try_pipe_rotor_scans_1d(). Restarting a project with pending 1D rotor scans raises AttributeError. Fixed in nd_scans_rebased by constructing the coordinator/planner before the loop (commit "Fix pipe_planner initialization order in Scheduler.init").

Fixed here — finalization crash

finalize_directed_scan_results did parse_nd_scan_energies_func(...)[0], but that parser returns the results dict itself — make_parser types it Optional[Dict], and every ESS adapter declares the same. This raised KeyError: 0 at finalization, i.e. after a multi-hour scan completed and before the rotor YAML was written, losing the run's output. The existing unit test's mock returned [mock_results], so it was written to match the bug rather than the real contract; I fixed the call site and added a regression test.

Design findings for discussion (independent review pass — flagging these to confirm, not asserting they're wrong)

• sampling_policy doesn't appear to have an input.yml surface. Its only writer (initialize_adaptive_scan_state) sits downstream of its only reader (is_adaptive_enabled), so the adaptive path looks unreachable from user input as written — the PR's own test sets the dict key by hand rather than going through any config path. Worth confirming whether there's a wiring path I'm missing.
• At ARC's default rotor_scan_resolution = 8.0, seed points (~285) already exceed ADAPTIVE_DEFAULT_MAX_POINTS = 200, so it looks like the adaptive phase never actually runs at default settings, and truncation in a0-major order would leave a large gap in one dimension. I also didn't see a convergence-based stopping criterion — only max_points_reached / grid_exhausted / full_coverage.
• The separability metric seemed to invert on adaptively-sampled data in a quick check: on a surface separable by construction it reported nonseparability ~0.26 / coupled / confidence 1.0, while the same surface on a dense grid scored 0.000000. The likely cause is estimating 1D marginals by averaging over points sharing an angle key, which degenerates when most angles are singletons (as they are under adaptive/sparse sampling). All current tests of this metric use complete factorial grids, so this gap wouldn't show up in CI as written.
• calculate_cross_term_strength returns calculate_nonseparability_score(...) directly, so in nonsep > 0.15 or cross_term > 0.10, COUPLING_NONSEP_THRESHOLD = 0.15 never ends up being the operative cut (both terms are the same value).
• Non-isomorphic points are routed to mark_scan_point_invalid but also still written to directed_scan via record_directed_scan_point. Downstream consumers read directed_scan and filter only on energy is not None, so an invalid point could shift the energy zero for the whole rotor rather than being excluded.
• Batch acquisition sorts candidates once without updating the sampled set between picks within a batch, so points within a single batch can collapse onto the same region.

Happy to discuss any of these — flagging them now since I had the branch open for the rebase anyway. Detailed PR for the rebase + fixes is #930.