Fail the run when an R statistics step fails or times out; draw PAC mosaics - #4
Merged
Merged
Conversation
On the FORGE treatment re-run, roi_directed (in all three source arms) and
roi_cross_freq's AAC/PPC tier were killed by a hard-coded one-hour R timeout.
Each module logged it and the command exited 0, so region tables from the
previous code version went on looking current. Only checking the outputs
found it.
- R statistics steps have no default time limit. They were hard-coded to
3600 s, or 600 s for the electrode and evoked modules; r_timeout_sec in a
module's config block sets one.
- A failed or timed-out step raises RStepFailed and the CLI exits 1. A run
over a paradigm, or over every paradigm, finishes its other modules first
and then lists what failed. A missing Rscript still only logs, as before.
- roi_cross_freq attempts PAC and AAC/PPC before failing, so one tier failing
does not cost the other its tables.
- roi_cross_freq's PAC mosaics named legacy columns (hedges_g, region,
contrast, freq_pair) that the native table does not have, so none was ever
drawn. They now read effect_size / spatial / hypothesis / band, and the
figures step draws them (summary() did only when figures were requested in
the same run).
The three vertex modules lose their limit too but still only log a failure;
that is left for the vertex split.
Tests: 230 pass (8 new), covering the missing default, a configured limit, a
timeout and a failure each raising, both cross-frequency tiers attempted, the
PAC mosaic columns and figures pass, and the CLI finishing a batch before
exiting 1.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RibM8Zep2YEjjUbc2LLkgj
alexedmon1
added a commit
that referenced
this pull request
Sep 11, 2026
A failed or timed-out R statistics step now fails the run, R steps have no default time limit (r_timeout_sec sets one), and roi_cross_freq draws its PAC mosaics (#4). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RibM8Zep2YEjjUbc2LLkgj
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
1 participant
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.
Summary
RStepFailed, exit 1). A paradigm-wide run finishes its other modules first, then lists what failed.r_timeout_secin a module's config block sets one.roi_cross_freqattempts PAC and AAC/PPC before failing, so one tier failing no longer costs the other its tables.Why
On the FORGE treatment re-run (v0.7.0), four R steps were killed at exactly 3600 s, logged, and exited 0:
roi_directedroi_cross_freqAAC/PPC…_aac_region_hypotheses.csvleft at 8 categoriesOnly a manual check of the outputs caught it. Run uncapped on an idle machine, the same steps got through their heavy edge tier in about 10 minutes. So the kills came from the limit combined with three arms running concurrently, which is exactly the situation a fixed limit can't anticipate.
Behaviour changes
Rscriptstill only logs, as before. The figures-only limit (300 s) is unchanged.Test plan
PYTHONPATH=src pytest tests: 230 passed (8 new)roi_directed(3 arms) androi_cross_freq(cartesian) summary steps, and confirm 10/10 categories🤖 Generated with Claude Code
https://claude.ai/code/session_01RibM8Zep2YEjjUbc2LLkgj