[us-commits] [ehb54/ultrascan3] f1f016: somo/grpy: review fixes -- wrong model, stderr dea...

[emre brookes]

  Branch: refs/heads/ehb54-issue-1016-somo-merge-2
  Home:   https://github.com/ehb54/ultrascan3
  Commit: f1f016b8bd741b66eb80d86f0908e47f3ecfac17
      https://github.com/ehb54/ultrascan3/commit/f1f016b8bd741b66eb80d86f0908e47f3ecfac17
  Author: ehb54 <brookes at uthscsa.edu>
  Date:   2026-08-18 (Tue, 18 Aug 2026)

  Changed paths:
    M .github/workflows/somo-module-tests.yml
    M us_somo/develop/grpy/grpy_process.hpp
    M us_somo/develop/grpy/grpy_shell.hpp
    M us_somo/develop/grpy/tests/test_process.cpp
    M us_somo/develop/include/us_hydrodyn.h
    M us_somo/develop/src/us_hydrodyn.cpp
    M us_somo/develop/src/us_hydrodyn_grpy.cpp
    M us_somo/develop/src/us_hydrodyn_other.cpp

  Log Message:
  -----------
  somo/grpy: review fixes -- wrong model, stderr deadlock, split banners

>From the review on ehb54/ultrascan3#524.

Shell models were written from the WRONG MODEL. grpy_write_shell_model()
rebuilt the bead order from `bead_model`, which calc_grpy_hydro()'s setup
loop leaves holding the LAST selected model. With several models selected and
"save shell models" on, every model but the last had its shells built from
another structure's beads. The index-bounds check below it only caught that
when the bead counts happened to differ. It now uses
bead_models[ grpy_last_model_number ], which grpy_process_next() sets per
model, with a bounds guard.

A child writing to stderr could DEADLOCK the run. run_program() drained
stdout in its poll loop but read stderr only after waitForFinished( -1 ). The
channels are separate and each pipe buffer is finite, so a child that writes
more than that buffer to stderr blocks in write(), stops producing stdout,
the loop stops advancing, and the wait -- which has no timeout -- never
returns. stderr is now drained every pass.

Progress text could reach the report. consume_progress() treated each read as
whole records, but reads off a pipe break wherever the buffer fills: a banner
split across two reads matched nothing and had both halves appended to the
report, which is what lands in .grpy_res and what parse_report() reads. It
now holds a trailing partial record back until its terminator arrives, which
also repairs a "\r\n" straddling the boundary. Covered by a new test that
fails against the old behaviour.

A user's Stop was reported as a memory failure. ShellReport::annotation()
treated levels == 0 as "even the smallest rung exceeded the available
memory", but levels == 0 is also reached when should_stop fires before the
first rung finishes. It now distinguishes stopped from mem_capped.

std::snprintf in grpy_shell.hpp worked only through transitive includes;
<cstdio> is now included.

Stale comments, all saying a running solve cannot be interrupted. It can:
ProcessSolver polls every 100 ms and kills the child, which the test "a
running solve is killed on stop" already asserted. Corrected in
us_hydrodyn_grpy.cpp, us_hydrodyn.cpp (which still described GRPY as
in-process), the ShellOptions::should_stop doc in grpy_shell.hpp -- which
contradicted the correct comment 270 lines below it -- and the user-facing
message in us_hydrodyn_other.cpp, which told the user the model would finish
first.

The grpy::ShellReport forward declaration in us_hydrodyn.h is removed: it is
referenced nowhere in that header, and its stated reason (keeping Eigen out)
no longer holds since the module dropped Eigen.

GRPY had NO CI coverage -- neither grpy/** nor us_hydrodyn_grpy.cpp appeared
in the paths filters and no job ran the suite. Both paths added, plus a job
running grpy/tests/run.sh. test_shell is Qt-free and runs; test_process needs
QtCore and run.sh skips it when QTDIR is unset. Verified on Linux/g++ 13.3.1
at -std=gnu++11 before adding: ALL PASS, exit 0.

Co-Authored-By: Claude Opus 5 <noreply at anthropic.com>



To unsubscribe from these emails, change your notification settings at https://github.com/ehb54/ultrascan3/settings/notifications