Repository navigation
Fix RA diagnostic plots: background them on Windows, log them everywhere - #60
Merged
Merged
Conversation
Two independent problems with how diagnostic_plots.py is invoked after each solve year. 1. The trailing '&' does not background anything on Windows. The generated run script is a .bat, and in cmd.exe '&' is a command separator, not a background operator -- so the plots run synchronously and block the solve loop. Measured at ~30s per solve year, ~2.0 min per run (~8.5% of wall clock) on a 4-solve-year case. Use `start /b` on Windows, gated on the existing LINUXORMAC global; Linux/macOS behaviour is unchanged. 2. diagnostic_plots.py never calls reeds.log.makelog, so its output never reaches gamslog.txt on any platform. gamslog.txt is written by each script's own FileHandler rather than by shell redirection, so a script that skips makelog is simply absent from the run log -- including its tracebacks. Since the call is also backgrounded and its exit code never checked, failures were completely silent. 24 other scripts appear in gamslog.txt; this one appeared zero times. Verified on Windows 10 / cmd.exe: a .bat using `start /b ""` returns in 163ms while the child runs on and completes, versus blocking for the child's full duration without it. With makelog added, a previously-silent invocation wrote 12 lines to gamslog.txt and surfaced a real traceback. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds the two inventory rows (runreeds.py, diagnostic_plots.py) and a section covering both the Windows backgrounding bug and the missing makelog call, with measured impact and what to re-check on the next upstream rebase. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
known-reeds-issues.md gains a symptom-level entry for the RA diagnostic-plot fixes, which the divergence log's header says every bug-fix section should have and this one lacked. The divergence log's Reference paragraph claimed both files had drifted upstream since the 2026.06.18 base and so needed re-authoring for an upstream PR. The base is now 2026.08.03, and on dev both files were byte-identical to that tag before this patch, so it applies there as-is. Both docs also cited runreeds.py by line number (959-967, 979-981, 987, 899-905). Those were already wrong on dev and this branch shifts them by another 7 lines, so they now name the code block instead. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The runreeds.py comment said only that cmd.exe has no '&' background operator, which reads as not applying to runs started from PowerShell. It does apply: on Windows the per-case .bat is launched via `start /wait cmd`, so it always runs under cmd.exe whatever shell started runreeds.py. The comment now says so. run_cepm.ps1's help text cited runreeds.py:959-967 and 899-905, which were already wrong on dev and have shifted further on this branch. It now names the code blocks instead, matching the CEPM docs. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Windows process launching and concurrent logging have not been validated in an end-to-end run after rebasing.
Review effort: Balanced
Findings: None
What changed in this PR
Fixes RA diagnostic plot execution so Windows runs remain non-blocking and plot output is logged across platforms.
Changes:
- Uses
start /bfor Windows background execution. - Initializes logging in
diagnostic_plots.py. - Documents the fixes and updates stale code references.
| File | Description |
|---|---|
runreeds.py |
Adds platform-specific background launching. |
run_cepm.ps1 |
Replaces brittle line-number references. |
reeds/resource_adequacy/diagnostic_plots.py |
Routes output and tracebacks to gamslog.txt. |
CEPM/reeds-to-cepm-log.md |
Records upstream divergence and rebase checks. |
CEPM/known-reeds-issues.md |
Documents the resolved symptoms and cause. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
ty-fi
added a commit
that referenced
this pull request
Sep 29, 2026
New CEPM/guidance/fixing-reeds-issues.md walks through fixing a bug in the ReEDS model code: branch from the working branch, record the issue in known-reeds-issues.md, fix and verify it, log any upstream-file changes in reeds-to-cepm-log.md, and open a pull request. Follows the layout of running-test-scenarios.md and uses fix/ra-plot-logging (PR #60) as a worked example. Indexed in CEPM/README.md. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
samhuestis
approved these changes
Sep 30, 2026
Collaborator
Author
|
HTML reports from pre- and post-RA fix: |
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.
Summary
Fixes two independent upstream bugs in how
reeds/resource_adequacy/diagnostic_plots.pyis invoked after each solve year.runreeds.pywrites a trailing&into the generated run script to background the plots. On Windows that script is a.bat, and incmd.exe&separates commands rather than backgrounding one, so the plots run synchronously. Measured at ~30 s per solve year, ~2.0 min per run (~8.5% of wall clock) on a 4-solve-year WECC-SW case. The fix usesstart /b ""on Windows, chosen by the existingLINUXORMACglobal. Linux and macOS behaviour is unchanged.gamslog.txt, on any platform. Each script writes togamslog.txtthrough its ownreeds.log.makeloghandler, not through shell redirection, anddiagnostic_plots.pynever called it. Its output and tracebacks were lost, and since its exit code is never checked, failures were silent. The fix adds the standardmakelogcall in__main__.Changes
runreeds.py: platform-specific background launch for the RA plot call insetup_sequential().reeds/resource_adequacy/diagnostic_plots.py:makelogcall.CEPM/reeds-to-cepm-log.md: inventory rows and a section for the two upstream-file changes, with checks to re-run on rebase. Also replaces stalerunreeds.pyline-number citations with named code blocks, and corrects an out-of-date note about upstream drift.CEPM/known-reeds-issues.md: new symptom-level entry, plus the same line-number cleanup.Testing
cmd.exe: a.batusingstart /b ""returned in 163 ms while the child process kept running and finished. Without it, the.batblocked for the child's full duration.makelogadded, a previously silent invocation wrote 12 lines togamslog.txt.dev, both Python files parse. No ReEDS case has been run on the rebased branch, so the end-to-end behaviour (astart /bline in the generated.bat,diagnostic_plots.py |lines ingamslog.txt) still needs checking on a real run.Follow-up
Adding
makelogsurfaced a real traceback fromdiagnostic_plots.py. This PR makes it visible but doesn't fix it, and it isn't yet recorded inknown-reeds-issues.md.Both bugs are present unchanged at upstream tag
2026.08.03, so they're candidates to contribute back.🤖 Generated with Claude Code