[us-commits] [ehb54/ultrascan3] 02a50a: grpy: review fixes from the merge PR -- wrong mode...

[emre brookes]

  Branch: refs/heads/port-524-review-fixes
  Home:   https://github.com/ehb54/ultrascan3
  Commit: 02a50af71d2db7759f7f1392e534ab0eaa061965
      https://github.com/ehb54/ultrascan3/commit/02a50af71d2db7759f7f1392e534ab0eaa061965
  Author: ehb54 <brookes at uthscsa.edu>
  Date:   2026-08-18 (Tue, 18 Aug 2026)

  Changed paths:
    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/grpy/tests/test_shell.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:
  -----------
  grpy: review fixes from the merge PR -- wrong model, stderr deadlock, split banners

Ports the review fixes made on ehb54/ultrascan3#524 back here, so somo-dev
and main do not diverge. Follows #525, which carried the C++11 correctness
work. Four real bugs and a set of stale comments.

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 catches that
when the bead counts happen to differ -- two models of equal size would each
have written a plausible-looking file built from the other's coordinates. 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; against
the old behaviour it reports the stage as "assembl" and appends "ing" to the
report, so the test fails there, as a test of this should.

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 -- the field comments say so, so the annotation
contradicted them. It now distinguishes stopped from mem_capped, with a
neutral third case.

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

Stale comments, all claiming 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, issue 972), 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.

Three unbraced bodies in test_shell.cpp are braced. They passed us3_style.pl
--check clean because it treated any '{' on the line as a body brace, so an
unbraced body whose argument is a braced initializer list read as clean;
fixed upstream in ehb54/grpy-cpp#2.

Verified here: 0 C++11 errors; test_shell and test_process both ALL PASS at
-std=gnu++11, including the new split-banner case; us3_style.pl --check
reports 0 violations on the module.

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