Skip to content

fix(wrxlib+ci): default WRX_RUN_OK=1, remove obsolete env-gate (0 skip)#166

Merged
k-yoshimi merged 2 commits into
developfrom
fix/wrxlib-test-remove-runok-gate
Apr 22, 2026
Merged

fix(wrxlib+ci): default WRX_RUN_OK=1, remove obsolete env-gate (0 skip)#166
k-yoshimi merged 2 commits into
developfrom
fix/wrxlib-test-remove-runok-gate

Conversation

@k-yoshimi
Copy link
Copy Markdown
Owner

@k-yoshimi k-yoshimi commented Apr 22, 2026

Summary

Two-half retirement of the obsolete WRX_RUN_OK env-gate that was once needed to avoid an SIGABRT in libgrf::grd1d. Both root causes are now fixed:

Half Commit Effect
Tests 9c44546d (test_wrxlib.py edit) RUN_OK = os.environ.get("WRX_RUN_OK", "1") != "0" (default-on, opt-out via =0). Sister files test_equivalence.py / test_sweep.py already used this style
CI 738316e2 (workflow edit) env: WRX_RUN_OK: "1" on the pytest job so CI explicitly exercises the run path

Why now

Reviewers

  • feature-dev:code-reviewer:
    • MED-1: dropped phantom WRX_REINIT_OK: "1" from the env block — WRX_REINIT_OK was retired at the Fortran level (wrx/wrcomm.f90:214) and is no longer read by any Python code. Fixed in commit 738316e2.
    • MED-2: python/wrxlib/README.md (5 stale references) and python/mcp-servers/wrx_mcp/README.md (§8) still tell users WRX_RUN_OK=1 is mandatory. Deferred to follow-up task — keeps this PR's scope tight (CI-behaviour-only).
  • Codex review: no findings. Re-ran wrxlib tests with WRX_RUN_OK=1 enabled and confirmed pass.

Test plan

  • pytest python/wrxlib/: 34 passed, 0 skipped (was 33 + 1 skip; test_run_and_get_state now runs and passes)
  • Wider regression pytest python/wrxlib/ python/totlib/ python/mcp-servers/wrx_mcp/: 115 passed, 0 skipped
  • CI pytest 3.11 / 3.13 green (the main delivery — verifying CI actually runs wrx_run without SEGV)
  • Cursor Bugbot review

Fallback if CI breaks

Per the reviewer's risk note: if a different gfortran-13.2 strictness issue surfaces, the symptom will be SIGABRT/SIGSEGV in a --forked child (not a suite-level crash). --maxfail=50 provides containment. If it happens: revert this PR, open follow-up to investigate via valgrind python -m pytest python/wrxlib/....

🤖 Generated with Claude Code


Note

Medium Risk
Changes CI/test gating so wrx_run() paths execute by default, which may surface native-library crashes or flakes in CI even though product code is unchanged.

Overview
CI now always exercises wrx_run() by default. The python-tests workflow sets WRX_RUN_OK=1 for the pytest step and drops the old rationale/notes about keeping run tests off in CI.

python/wrxlib/tests/test_wrxlib.py flips RUN_OK to default-on (WRX_RUN_OK=0 opt-out), updates skip messages/docs accordingly, and makes the TestWrxlibRun suite run whenever libwrxapi.so is present unless explicitly disabled.

Reviewed by Cursor Bugbot for commit 738316e. Bugbot is set up for automated code reviews on this repo. Configure here.

k-yoshimi and others added 2 commits April 22, 2026 19:27
The historical libwrxapi.so SEGV inside wrcalpwr -> grd1d was fixed in
PR #123 by setenv'ing WRX_NO_GRAPHICS=1 inside wrx_api_init. The opt-in
gate in test_wrxlib.py is now stale; convert to the default-on pattern
already used by sister test files (test_equivalence.py, test_sweep.py,
test_property_boundary.py): WRX_RUN_OK=0 still works as a kill switch.

Result: test_run_and_get_state runs by default. wrxlib subdir now reports
34 pass / 0 skip (was 33 pass / 1 skip).

Note: CI workflow comment at .github/workflows/python-tests.yml:229-235
still warns "Don't default WRX_RUN_OK on" — that comment was written
before PR #123 landed; the SEGV root cause is now fixed. If CI surfaces
a regression, revisit the comment alongside this change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The original "Don't default WRX_RUN_OK on" CI rationale was an
SIGABRT-avoidance gate predating PR #123 (wrx_api_init sets
WRX_NO_GRAPHICS=1 to suppress libgrf::grd1d) and PR #163 (eq EQFINI
rearms eq_bpsd_init_flag SAVE so suite-level reinit cycles stop
SEGV'ing). With both fixes in place the gate is obsolete and CI
should exercise the full wrx_run() surface — leaving it gated
would let regressions in wrx run-path code reach develop without
ever being exercised by CI.

This is the CI half of the gate retirement; the test-side
change (test_wrxlib.py defaulting RUN_OK=on) was the previous
commit on this branch.

Codex review: no findings.
feature-dev review: dropped phantom WRX_REINIT_OK from the env
  block (no Python code reads it; the gate was retired at the
  Fortran level per wrx/wrcomm.f90:214). README doc-drift
  cleanup deferred to follow-up task.

Verified locally:
- pytest python/wrxlib/ : 34 passed, 0 skipped
- (CI behavior change is the actual delivery — local already
  defaults RUN_OK=on after the previous commit)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@k-yoshimi
Copy link
Copy Markdown
Owner Author

@cursor review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 738316e. Configure here.

@k-yoshimi k-yoshimi merged commit abab96d into develop Apr 22, 2026
3 checks passed
@k-yoshimi k-yoshimi deleted the fix/wrxlib-test-remove-runok-gate branch April 22, 2026 21:17
k-yoshimi added a commit that referenced this pull request Apr 23, 2026
Align wrxlib/README.md, wrx_mcp/README.md + server.py, and the 3
wrxlib/examples/*.py docstrings with the current state of the
WRX_RUN_OK gate:

* PR #123 fixed the libgrf::grd1d SEGV via WRX_NO_GRAPHICS=1 setenv
  in wrx_api_init, so wrx_run no longer crashes through dlopen.
* PR #163 added EQFINI rearm so reinit cycles are also safe.
* PR #166 flipped the test-side WRX_RUN_OK gate to default-on
  (os.environ.get("WRX_RUN_OK", "1") != "0").

The wrxlib library itself has no runtime gate — .run() dispatches
unconditionally. The MCP-level gate in wrx_mcp/server.py is retained
as a defence-in-depth / explicit opt-in safeguard so an LLM does not
accidentally burn minutes on a ray-tracing run without the user
authorising it; the gate error message, module docstring, run /
run_and_get_state tool docstrings, and --help text were updated to
reflect this rationale instead of the obsolete "SEGV prevention"
story.

No behavioural change — docs + gate message strings only.
wrx_mcp pytest: 37/37 PASS under --forked --timeout=120.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
k-yoshimi added a commit that referenced this pull request Apr 27, 2026
…s + ray helper

Extends the tr/eq applications pattern to wrx (extended ray tracing).
Three samples adapted to wrx's per-ray array indexing:

1. safe_run + ray-launch sanity report — pure-Python preflight checks
   each ray's RPI/RF/ANGT against [RR-(RA+RB), RR+(RA+RB)] etc.; on
   WrxlibRunError it prints which ray is the suspect (since geometry-
   error retry is futile).
2. fan_sweep via _apply_rays — fan_sweep([8,10,12]) returns pwr_tot,
   pwr_nsa (per-species sum), and pwr_nsa_nray (per-ray x per-species
   2D matrix).
3. auto_setup + hand-rolled _hand_validate + DEVICE_PRESETS[
   "ITER_LHCD_2ray"] — preset splits {shape, rays}; validate enforces
   NSMAX>=1, non-empty rays, REQUIRED_RAY_KEYS = ("RF","RPI","ZPI",
   "ANGT") per ray. NRAYMAX = len(rays) auto-derived.

The _apply_rays(wrx, rays) helper takes [{"RF":170e3, "RPI":8.0,
"ZPI":0.5, "ANGT":10.0}, ...], hides 1-origin Fortran indexing,
fills defaults (PHII=0, ANGPHI=0, UU=1, MODEW=1), and writes
RFIN[i]/RPIN[i]/... via set_param.

Reproduced live numeric outputs against libwrxapi.so (ITER 170 GHz,
NSMAX=2 D+e):
  Single ray (ANGT=10):  pwr_tot=0.7840, pwr_nsa=[0.7840, 0.0]
  3-ray fan ANGT=8/10/12: pwr_tot=1.9122
  2-ray ITER preset:     pwr_tot=1.0073
  Bad geometry (RPI=1.0 < 2.0): preflight flags ray 1 then ierr=3

Caveats noted in {note} admonition: historical libgrf::grd1d SEGV
(retired by PR #123), WRX_RUN_OK=1 default since PR #166, and the
pwr_nray[i]==0.0 quirk (use pwr_nsa_nray row sums).

Sphinx wrx-ja build: warning-free (-W --keep-going passes).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
k-yoshimi added a commit that referenced this pull request Apr 27, 2026
English version of the wrx applications page (safe_run / fan_sweep /
auto_setup + _apply_rays helper). Mirrors 3b86091. Numeric outputs
(pwr_tot=1.9122, pwr_tot=1.0073, etc.) and PR #123 / PR #166 caveat
references preserved verbatim.

Verified: sphinx wrx-en build is warning-free.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant