pipe: carry rotor scan resolution through the task payload - #1027
Conversation
The rotor_scan_1d TaskSpec held no scan_res, so a piped ASE/MLIP scan ran the worker node's rotor_scan_resolution setting instead of ARC's intended one. Capture it at staging time and forward it in the worker as trsh scan_res.
There was a problem hiding this comment.
Pull request overview
This pull request fixes a silent correctness issue in piped 1D rotor scans by ensuring the intended rotor scan resolution is preserved across the pipe boundary (staging → worker execution), rather than implicitly falling back to whatever resolution is configured on the worker node.
Changes:
- Capture
scan_resduring task staging inbuild_rotor_scan_1d_tasksand store it in eachrotor_scan_1dtask payload. - Forward
scan_reson the worker side via the existing troubleshooting override channelargs['trsh']['scan_res']. - Add targeted unit tests covering both staging-time capture and worker-time forwarding / fallback behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| arc/scripts/pipe_worker.py | Extracts scan_res from rotor_scan_1d payload and forwards it via args['trsh']['scan_res'] when present. |
| arc/scripts/pipe_worker_test.py | Adds unit tests for _get_family_extra_kwargs to ensure scan_res forwarding and correct omission when absent. |
| arc/job/pipe/pipe_run.py | Extends build_rotor_scan_1d_tasks to accept optional scan_res and include it in the task payload. |
| arc/job/pipe/pipe_run_test.py | Adds tests to confirm scan_res is carried into payload when provided and omitted when None. |
| arc/job/pipe/pipe_planner.py | Passes the staging-side rotor scan resolution into build_rotor_scan_1d_tasks so workers don’t revert to their local defaults. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1027 +/- ##
==========================================
+ Coverage 64.77% 64.83% +0.05%
==========================================
Files 119 119
Lines 40011 40015 +4
Branches 10341 10342 +1
==========================================
+ Hits 25918 25944 +26
+ Misses 11110 11088 -22
Partials 2983 2983
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:
|
Motivation
A piped 1D rotor scan (
rotor_scan_1d, the path MLIP/ASE rotors take) ran at whateverrotor_scan_resolutionthe worker node's settings happened to hold — not the resolution the runintended. The
TaskSpeccarried noscan_res, so the value simply rebound on the compute node.This is the same silent-wrong-answer class as #1019: below 18 points per rotor RMG-Py's Fourier
fitter never runs and
get_potential()reads an uninitialised C double. Nothing crashes; thetorsional contribution is just wrong. #1019 makes resolution a property of the run at the input
layer; this closes the remaining hole, where the value was lost crossing the pipe boundary.
What changed
Capture the resolution at staging time, on the machine that knows what the run asked for, and
forward it in the worker:
pipe_planner.py/pipe_run.py—build_rotor_scan_1d_tasksrecordsscan_resinto therotor_scan_1dtask payload.pipe_worker.py— forwards it asargs['trsh']['scan_res'], the existing per-job overridechannel (the same one Add a rotor_scan_resolution input key #1019 uses), so no new mechanism is introduced.
Absent a
scan_resin the payload, behaviour is unchanged: the worker's settings default stillapplies, exactly as before.
Tests
New coverage in
pipe_run_test.pyandpipe_worker_test.pypinning that the resolution iscaptured at staging and forwarded at execution.
arc/job/pipe/: 157 passed.arc/job/adapters/ase_test.py arc/job/adapters/common_test.py arc/scheduler_test.py: 86 passed.Run under
arc_envwithHOMEpointed at an empty dir (ARC's suite reads~/.arc/settings.py),-n0 -o addopts="".An end-to-end check against the benchmark: an ethane pipe scan matched the reference potential to
0.0279 kJ/mol.
Relationship to other PRs
Independent of #1019 and mergeable on its own — they close the same gap at two different layers
(#1019 at the input/scheduler boundary for ESS
scanjobs, this at the pipe boundary for pipedrotor tasks). Rebased onto
mainnow that #1018 (the one-process ASE relaxed scan this builds on)has merged.