benchmarks: settle the argv contract for the whole suites/ tree (#489) - #491

Merged
wshlavacek merged 2 commits into
mainfrom
bench-argv-entry-points-489
Aug 27, 2026
Merged

benchmarks: settle the argv contract for the whole suites/ tree (#489)#491
wshlavacek merged 2 commits into
mainfrom
bench-argv-entry-points-489

Conversation

@wshlavacek

Copy link
Copy Markdown
Collaborator

Closes#489. Follows #490, which fixed
the two runners run_all.py can reach.

The paper side's comment on the issue is followed exactly: _ssa_cell.py and
bngsim_wrapper.py get no parser, that exemption is now endorsed by a test
rather than left to the next implementer, and the freedom granted over
recover_s4_points.py is used.

The hazard was demonstrated, not argued

Two of the seven wrote committed files on a bare invocation. Reproduced
against this repository, then restored from git:

$ cd benchmarks/suites/jacobian && python probe_attach.py --help
--help: attached=None n_func_rxns=None ns=None self_check_failed=None err=missing
WROTE results/attach_probe.json
$ git diff --stat -- results/attach_probe.json
.../jacobian/results/attach_probe.json | 48 +---------------------
1 file changed, 2 insertions(+), 46 deletions(-)

46 lines of measurement replaced by [{"model": "--help", "error": "missing"}]
by the command typed to find out what the script does. That file is the record of
which models genuinely attach the analytical Jacobian, which is what decides
membership in the suite's speedup table.

scriptwhat --help used to do
jacobian/probe_attach.pyprobed a model named --help; overwrote the committedresults/attach_probe.json
ode_engines_s4_sbml/check_sbml_engine_agreement.pyran bngsim + RoadRunner + COPASI over the Table S4 models, then overwrote its committed report
ode_fullnet/recover_s4_points.pyregenerated two networks through BNG2.pl, rewrote the characterization under parity_checks/, then copied both into another repository'slatex/generated/
ssa_table5/convert_all.pyreconverted all 14 models, rewrote results/converted/ + conversion_log.json
ssa_table5/emit_ssa_table.pyrewrote ssa_timing_ballpark.json in place and re-rendered the .md
ssa_table5/merge_jobout.pyrebuilt ssa_timing_ballpark.json from _jobout/
jacobian/diagnose_divergence.pyread --help as a model id and stopped — the one benign case

The fix

All seven build an ArgumentParser: --help exits 0 with usage, an unrecognized
flag exits 2, a bare invocation is unchanged.

Where a probe needs somewhere harmless to write, it now has a flag — the same
shape --out took on the PSA companion in #490:

  • --out on probe_attach.py and check_sbml_engine_agreement.py;
  • --paper-dir / --no-copy on recover_s4_points.py. That one also skips
    rather than crashes
    when the paper checkout is absent: the merge has already
    happened by then, so a FileNotFoundError at the copy left the work done and
    the run marked failed. Its stale /Users/wish/Code/PyBNF/... follow-up line is
    gone too, per your note that it was never a contract.

diagnose_divergence.py is both driver and worker — it re-enters itself as
--worker on an argv it composes in _spawn(). That branch is matched before
the parser and left byte-identical; only the half a person types is parsed.

The exemption, endorsed

ssa_table5/_ssa_cell.py (spawned per (engine, model) by run_ssa_timing.py,
Table 5's measurement path) and sbml_test_suite/testrunner/bngsim_wrapper.py
(invoked by the vendored runner on %d %n %o %l %v) get no parser. Each now
names its caller in a comment saying why there is no parser.

Worth noting: bngsim_wrapper.py was already correct. Its argc check answers
--help with a usage line and exit 2 — the issue's table overstated it as
ignoring argv. It needed the comment, not a change.

The endorsement is a test, not a convention:

deftest_spawned_workers_keep_their_positional_contract(rel):
src= (SUITES_DIR/rel).read_text()
assert"argparse"notinsrc, f"{rel} grew a parser -- it is spawned by ..."assertSPAWNED_WORKERS[rel] insrc, f"{rel} does not name its caller"

So the next argv sweep cannot quietly "fix" these two, and cannot read the
missing parser as an oversight either.

The test now discovers its family

test_benchmark_runner_help.py no longer derives from run_all.py's registry
plus a hand-kept companion list. It globs every suites/ script with a
__main__ guard
and subtracts SPAWNED_WORKERS — 59 scripts, 127 cases,
15 s. A script added to any suite inherits the contract without an edit here.
A separate test asserts every registry-named script lands inside that family, so
the orchestrator link stays explicit.

Two tests assert --help leaves each committed artifact byte-identical, which
states the actual harm rather than only "exit 0".

Verification

  • 127 cases pass; full local suite green.
  • Re-ran every script's --help with libsbml, scipy, roadrunner, amici,
    COPASI, antimony, gillespy2, diffrax, jax, pandas, matplotlib and
    psutil blocked at import. Only two scripts import an optional engine at module
    scope — biomodels/filter.py (libsbml) and showcase/run_ode_trf_fit_from_net.py
    (scipy) — and both deps are guaranteed present wherever this test runs
    (python-libsbml is a core dependency; scipy comes in via the test extra,
    which dev includes and CI syncs).
  • diagnose_divergence.py --worker verified to still reach worker() and fail
    inside it on a bogus path, proving the spawn argv unpack is intact.
  • The two committed artifacts are byte-identical in this branch.

Risk

None to any measured value. Every script behaves identically when invoked as it
was before, and the two scripts on a measurement path are the two that were not
touched functionally.

#488 fixed the two runners the orchestrator can reach. This finishes the
sweep over the seven scripts it cannot, and the finding is that the
hazard was demonstrated rather than argued:
* jacobian/probe_attach.py read `--help` as a model id, failed to find
a model by that name, and wrote the one-row failure over the
COMMITTED results/attach_probe.json -- 46 lines of measurement
replaced by two, by a command typed to find out what the script
does. Reproduced against the file in this repository.
* ode_engines_s4_sbml/check_sbml_engine_agreement.py ran bngsim,
RoadRunner and COPASI over the Table S4 models, then overwrote its
committed report.
* ode_fullnet/recover_s4_points.py reached furthest: regenerate two
networks through BNG2.pl, rewrite the characterization under
parity_checks/, then copy both files into a DIFFERENT repository's
latex/generated/, one of them committed there.
* ssa_table5/{convert_all,emit_ssa_table,merge_jobout}.py reconverted
the corpus or rewrote the Table 5 result set.
* jacobian/diagnose_divergence.py read `--help` as a model id (and
stopped there, the one benign case).
All seven now build an ArgumentParser: `--help` exits 0 with usage, an
unrecognized flag exits 2, a bare invocation is unchanged. Where a probe
needs somewhere harmless to write, it gets a flag -- `--out` on
probe_attach.py and check_sbml_engine_agreement.py, `--paper-dir` /
`--no-copy` on recover_s4_points.py, which also skips rather than
crashes when the paper checkout is absent. diagnose_divergence.py
re-enters itself as `--worker` on an argv it composes; that half is
matched before the parser and left exactly as it was.
Two scripts are exempt, at the paper side's explicit ask on the issue:
ssa_table5/_ssa_cell.py is spawned per (engine, model) by
run_ssa_timing.py, which is Table 5's measurement path, and
bngsim_wrapper.py is invoked by the vendored SBML test-suite runner on
the fixed `%d %n %o %l %v` template. A parser in either changes a
caller's contract to buy a usage line for an invocation nobody makes.
Each names its caller instead, and the wrapper already refused a hand
invocation with usage + exit 2.
test_benchmark_runner_help.py now discovers its family by structure --
every suites/ script with a `__main__` guard, minus that exemption list
-- rather than from run_all.py's registry plus a hand-kept companion
list. 127 cases, 15 s. A script added to any suite inherits the contract
without an edit here, and the exemptions are themselves tested: no
parser, and the worker must still name its caller, so the omission reads
as intent. Two further tests assert `--help` leaves each committed
artifact byte-identical.
Verified with libsbml, scipy, roadrunner, amici, COPASI and antimony
blocked at import: only the two scripts that import an optional engine
at module scope are affected, and both of those deps (python-libsbml
core, scipy via the test extra) are present wherever this test runs.
No measured value changes.
CI was red on 19 cases, all in the new test file, all one defect it had
just been widened enough to see.
Eight scripts resolved their repo root as
BNGSIM = Path(os.environ.get("BNGSIM_ROOT", Path.home() / "Code" / "bngsim"))
and then put <root>/parity_checks and <root>/parity_checks/bng_parity on
sys.path. So `import _bng_common` at module scope resolved only on a
machine whose clone happens to sit at ~/Code/bngsim, and raised
ModuleNotFoundError in any other checkout -- a worktree, a fresh clone,
and every CI runner. `--help` on those eight could not answer because the
module could not import:
ode_engines_s3/run_s3_timing.py (and patch_amici_split.py, which
imports it)
ode_engines_s4_sbml/run_s4_timing.py
ode_engines_s4_sbml/check_sbml_engine_agreement.py
ode_fullnet/{run_timing,run_forced,gen_networks,probe_growth,
recover_s4_points}.py
The default is now Path(__file__).resolve().parents[3] -- the checkout the
file lives in -- which is the idiom biomodels/emit.py already uses
(`BNGSIM_ROOT = BENCH_DIR.parents[2]`). BNGSIM_ROOT still overrides, so
the case the env var was written for is untouched: a different venv run
against a canonical checkout. Where the two already coincided, the
resolved path is identical and nothing moves.
The test could not have caught this, because the author's $HOME made the
wrong default look right. Every probe now runs with HOME (and USERPROFILE)
pointed at an empty session-scoped directory, so "anyone can run --help"
means anyone. Verified by stashing one script's fix: the two cases for it
fail locally with the same ModuleNotFoundError CI reported.
Also verified with amici, COPASI, gillespy2, diffrax, basico, pybnf,
psutil, matplotlib and bioservices blocked at import and BNGPATH /
SBML_TEST_SUITE_DIR cleared, alongside the scrubbed HOME: all 59 scripts
answer --help.
@wshlavacek

Copy link
Copy Markdown
CollaboratorAuthor

CI was red — 19 cases, all in the new test file, all one defect it had just been widened enough to see. Fixed in c031852.

Eight scripts resolved their repo root as Path.home() / "Code" / "bngsim" and then put <root>/parity_checks on sys.path. Their module-scope import _bng_common therefore resolved only on a machine whose clone sits at that path, and raised ModuleNotFoundError in any other checkout — a worktree, a fresh clone, every CI runner:

ode_engines_s3/run_s3_timing.py (+ patch_amici_split.py, which imports it)
ode_engines_s4_sbml/run_s4_timing.py
ode_engines_s4_sbml/check_sbml_engine_agreement.py
ode_fullnet/{run_timing,run_forced,gen_networks,probe_growth,recover_s4_points}.py

The default is now Path(__file__).resolve().parents[3] — the checkout the file lives in — which is the idiom biomodels/emit.py already uses. BNGSIM_ROOT still overrides, so the case the env var was written for (a different venv run against a canonical checkout) is untouched, and where the two already coincided the resolved path is identical.

The test could not have caught it, because the author's $HOME made the wrong default look right. Every probe now runs with HOME/USERPROFILE pointed at an empty session-scoped directory. Stashing one script's fix reproduces the exact CI failure locally, so this class is now caught on a dev machine rather than on a runner.

Worth noting this is the same shape as the issue itself: a script that only works where its author's files happen to be. It just showed up in sys.path instead of in an output path.

@wshlavacek
wshlavacek merged commit 89c2a5c into mainAug 27, 2026
4 checks passed
@wshlavacek
wshlavacek deleted the bench-argv-entry-points-489 branch August 27, 2026 16:46
Sign up for freeto 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.

benchmarks/suites: nine scripts outside run_all.py's registry still ignore argv

1 participant

@wshlavacek
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

benchmarks: settle the argv contract for the whole suites/ tree (#489) - #491

Merged
wshlavacek merged 2 commits into
mainfrom
bench-argv-entry-points-489
Aug 27, 2026
Merged

benchmarks: settle the argv contract for the whole suites/ tree (#489)#491
wshlavacek merged 2 commits into
mainfrom
bench-argv-entry-points-489

Conversation

@wshlavacek

Copy link
Copy Markdown
Collaborator

Closes#489. Follows #490, which fixed
the two runners run_all.py can reach.

The paper side's comment on the issue is followed exactly: _ssa_cell.py and
bngsim_wrapper.py get no parser, that exemption is now endorsed by a test
rather than left to the next implementer, and the freedom granted over
recover_s4_points.py is used.

The hazard was demonstrated, not argued

Two of the seven wrote committed files on a bare invocation. Reproduced
against this repository, then restored from git:

$ cd benchmarks/suites/jacobian && python probe_attach.py --help
--help: attached=None n_func_rxns=None ns=None self_check_failed=None err=missing
WROTE results/attach_probe.json
$ git diff --stat -- results/attach_probe.json
.../jacobian/results/attach_probe.json | 48 +---------------------
1 file changed, 2 insertions(+), 46 deletions(-)

46 lines of measurement replaced by [{"model": "--help", "error": "missing"}]
by the command typed to find out what the script does. That file is the record of
which models genuinely attach the analytical Jacobian, which is what decides
membership in the suite's speedup table.

scriptwhat --help used to do
jacobian/probe_attach.pyprobed a model named --help; overwrote the committedresults/attach_probe.json
ode_engines_s4_sbml/check_sbml_engine_agreement.pyran bngsim + RoadRunner + COPASI over the Table S4 models, then overwrote its committed report
ode_fullnet/recover_s4_points.pyregenerated two networks through BNG2.pl, rewrote the characterization under parity_checks/, then copied both into another repository'slatex/generated/
ssa_table5/convert_all.pyreconverted all 14 models, rewrote results/converted/ + conversion_log.json
ssa_table5/emit_ssa_table.pyrewrote ssa_timing_ballpark.json in place and re-rendered the .md
ssa_table5/merge_jobout.pyrebuilt ssa_timing_ballpark.json from _jobout/
jacobian/diagnose_divergence.pyread --help as a model id and stopped — the one benign case

The fix

All seven build an ArgumentParser: --help exits 0 with usage, an unrecognized
flag exits 2, a bare invocation is unchanged.

Where a probe needs somewhere harmless to write, it now has a flag — the same
shape --out took on the PSA companion in #490:

  • --out on probe_attach.py and check_sbml_engine_agreement.py;
  • --paper-dir / --no-copy on recover_s4_points.py. That one also skips
    rather than crashes
    when the paper checkout is absent: the merge has already
    happened by then, so a FileNotFoundError at the copy left the work done and
    the run marked failed. Its stale /Users/wish/Code/PyBNF/... follow-up line is
    gone too, per your note that it was never a contract.

diagnose_divergence.py is both driver and worker — it re-enters itself as
--worker on an argv it composes in _spawn(). That branch is matched before
the parser and left byte-identical; only the half a person types is parsed.

The exemption, endorsed

ssa_table5/_ssa_cell.py (spawned per (engine, model) by run_ssa_timing.py,
Table 5's measurement path) and sbml_test_suite/testrunner/bngsim_wrapper.py
(invoked by the vendored runner on %d %n %o %l %v) get no parser. Each now
names its caller in a comment saying why there is no parser.

Worth noting: bngsim_wrapper.py was already correct. Its argc check answers
--help with a usage line and exit 2 — the issue's table overstated it as
ignoring argv. It needed the comment, not a change.

The endorsement is a test, not a convention:

deftest_spawned_workers_keep_their_positional_contract(rel):
src= (SUITES_DIR/rel).read_text()
assert"argparse"notinsrc, f"{rel} grew a parser -- it is spawned by ..."assertSPAWNED_WORKERS[rel] insrc, f"{rel} does not name its caller"

So the next argv sweep cannot quietly "fix" these two, and cannot read the
missing parser as an oversight either.

The test now discovers its family

test_benchmark_runner_help.py no longer derives from run_all.py's registry
plus a hand-kept companion list. It globs every suites/ script with a
__main__ guard
and subtracts SPAWNED_WORKERS — 59 scripts, 127 cases,
15 s. A script added to any suite inherits the contract without an edit here.
A separate test asserts every registry-named script lands inside that family, so
the orchestrator link stays explicit.

Two tests assert --help leaves each committed artifact byte-identical, which
states the actual harm rather than only "exit 0".

Verification

  • 127 cases pass; full local suite green.
  • Re-ran every script's --help with libsbml, scipy, roadrunner, amici,
    COPASI, antimony, gillespy2, diffrax, jax, pandas, matplotlib and
    psutil blocked at import. Only two scripts import an optional engine at module
    scope — biomodels/filter.py (libsbml) and showcase/run_ode_trf_fit_from_net.py
    (scipy) — and both deps are guaranteed present wherever this test runs
    (python-libsbml is a core dependency; scipy comes in via the test extra,
    which dev includes and CI syncs).
  • diagnose_divergence.py --worker verified to still reach worker() and fail
    inside it on a bogus path, proving the spawn argv unpack is intact.
  • The two committed artifacts are byte-identical in this branch.

Risk

None to any measured value. Every script behaves identically when invoked as it
was before, and the two scripts on a measurement path are the two that were not
touched functionally.

#488 fixed the two runners the orchestrator can reach. This finishes the
sweep over the seven scripts it cannot, and the finding is that the
hazard was demonstrated rather than argued:
* jacobian/probe_attach.py read `--help` as a model id, failed to find
a model by that name, and wrote the one-row failure over the
COMMITTED results/attach_probe.json -- 46 lines of measurement
replaced by two, by a command typed to find out what the script
does. Reproduced against the file in this repository.
* ode_engines_s4_sbml/check_sbml_engine_agreement.py ran bngsim,
RoadRunner and COPASI over the Table S4 models, then overwrote its
committed report.
* ode_fullnet/recover_s4_points.py reached furthest: regenerate two
networks through BNG2.pl, rewrite the characterization under
parity_checks/, then copy both files into a DIFFERENT repository's
latex/generated/, one of them committed there.
* ssa_table5/{convert_all,emit_ssa_table,merge_jobout}.py reconverted
the corpus or rewrote the Table 5 result set.
* jacobian/diagnose_divergence.py read `--help` as a model id (and
stopped there, the one benign case).
All seven now build an ArgumentParser: `--help` exits 0 with usage, an
unrecognized flag exits 2, a bare invocation is unchanged. Where a probe
needs somewhere harmless to write, it gets a flag -- `--out` on
probe_attach.py and check_sbml_engine_agreement.py, `--paper-dir` /
`--no-copy` on recover_s4_points.py, which also skips rather than
crashes when the paper checkout is absent. diagnose_divergence.py
re-enters itself as `--worker` on an argv it composes; that half is
matched before the parser and left exactly as it was.
Two scripts are exempt, at the paper side's explicit ask on the issue:
ssa_table5/_ssa_cell.py is spawned per (engine, model) by
run_ssa_timing.py, which is Table 5's measurement path, and
bngsim_wrapper.py is invoked by the vendored SBML test-suite runner on
the fixed `%d %n %o %l %v` template. A parser in either changes a
caller's contract to buy a usage line for an invocation nobody makes.
Each names its caller instead, and the wrapper already refused a hand
invocation with usage + exit 2.
test_benchmark_runner_help.py now discovers its family by structure --
every suites/ script with a `__main__` guard, minus that exemption list
-- rather than from run_all.py's registry plus a hand-kept companion
list. 127 cases, 15 s. A script added to any suite inherits the contract
without an edit here, and the exemptions are themselves tested: no
parser, and the worker must still name its caller, so the omission reads
as intent. Two further tests assert `--help` leaves each committed
artifact byte-identical.
Verified with libsbml, scipy, roadrunner, amici, COPASI and antimony
blocked at import: only the two scripts that import an optional engine
at module scope are affected, and both of those deps (python-libsbml
core, scipy via the test extra) are present wherever this test runs.
No measured value changes.
CI was red on 19 cases, all in the new test file, all one defect it had
just been widened enough to see.
Eight scripts resolved their repo root as
BNGSIM = Path(os.environ.get("BNGSIM_ROOT", Path.home() / "Code" / "bngsim"))
and then put <root>/parity_checks and <root>/parity_checks/bng_parity on
sys.path. So `import _bng_common` at module scope resolved only on a
machine whose clone happens to sit at ~/Code/bngsim, and raised
ModuleNotFoundError in any other checkout -- a worktree, a fresh clone,
and every CI runner. `--help` on those eight could not answer because the
module could not import:
ode_engines_s3/run_s3_timing.py (and patch_amici_split.py, which
imports it)
ode_engines_s4_sbml/run_s4_timing.py
ode_engines_s4_sbml/check_sbml_engine_agreement.py
ode_fullnet/{run_timing,run_forced,gen_networks,probe_growth,
recover_s4_points}.py
The default is now Path(__file__).resolve().parents[3] -- the checkout the
file lives in -- which is the idiom biomodels/emit.py already uses
(`BNGSIM_ROOT = BENCH_DIR.parents[2]`). BNGSIM_ROOT still overrides, so
the case the env var was written for is untouched: a different venv run
against a canonical checkout. Where the two already coincided, the
resolved path is identical and nothing moves.
The test could not have caught this, because the author's $HOME made the
wrong default look right. Every probe now runs with HOME (and USERPROFILE)
pointed at an empty session-scoped directory, so "anyone can run --help"
means anyone. Verified by stashing one script's fix: the two cases for it
fail locally with the same ModuleNotFoundError CI reported.
Also verified with amici, COPASI, gillespy2, diffrax, basico, pybnf,
psutil, matplotlib and bioservices blocked at import and BNGPATH /
SBML_TEST_SUITE_DIR cleared, alongside the scrubbed HOME: all 59 scripts
answer --help.
@wshlavacek

Copy link
Copy Markdown
CollaboratorAuthor

CI was red — 19 cases, all in the new test file, all one defect it had just been widened enough to see. Fixed in c031852.

Eight scripts resolved their repo root as Path.home() / "Code" / "bngsim" and then put <root>/parity_checks on sys.path. Their module-scope import _bng_common therefore resolved only on a machine whose clone sits at that path, and raised ModuleNotFoundError in any other checkout — a worktree, a fresh clone, every CI runner:

ode_engines_s3/run_s3_timing.py (+ patch_amici_split.py, which imports it)
ode_engines_s4_sbml/run_s4_timing.py
ode_engines_s4_sbml/check_sbml_engine_agreement.py
ode_fullnet/{run_timing,run_forced,gen_networks,probe_growth,recover_s4_points}.py

The default is now Path(__file__).resolve().parents[3] — the checkout the file lives in — which is the idiom biomodels/emit.py already uses. BNGSIM_ROOT still overrides, so the case the env var was written for (a different venv run against a canonical checkout) is untouched, and where the two already coincided the resolved path is identical.

The test could not have caught it, because the author's $HOME made the wrong default look right. Every probe now runs with HOME/USERPROFILE pointed at an empty session-scoped directory. Stashing one script's fix reproduces the exact CI failure locally, so this class is now caught on a dev machine rather than on a runner.

Worth noting this is the same shape as the issue itself: a script that only works where its author's files happen to be. It just showed up in sys.path instead of in an output path.

@wshlavacek
wshlavacek merged commit 89c2a5c into mainAug 27, 2026
4 checks passed
@wshlavacek
wshlavacek deleted the bench-argv-entry-points-489 branch August 27, 2026 16:46
Sign up for freeto 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.

benchmarks/suites: nine scripts outside run_all.py's registry still ignore argv

1 participant

@wshlavacek
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

benchmarks: settle the argv contract for the whole suites/ tree (#489) - #491

Merged
wshlavacek merged 2 commits into
mainfrom
bench-argv-entry-points-489
Aug 27, 2026
Merged

benchmarks: settle the argv contract for the whole suites/ tree (#489)#491
wshlavacek merged 2 commits into
mainfrom
bench-argv-entry-points-489

Conversation

@wshlavacek

Copy link
Copy Markdown
Collaborator

Closes#489. Follows #490, which fixed
the two runners run_all.py can reach.

The paper side's comment on the issue is followed exactly: _ssa_cell.py and
bngsim_wrapper.py get no parser, that exemption is now endorsed by a test
rather than left to the next implementer, and the freedom granted over
recover_s4_points.py is used.

The hazard was demonstrated, not argued

Two of the seven wrote committed files on a bare invocation. Reproduced
against this repository, then restored from git:

$ cd benchmarks/suites/jacobian && python probe_attach.py --help
--help: attached=None n_func_rxns=None ns=None self_check_failed=None err=missing
WROTE results/attach_probe.json
$ git diff --stat -- results/attach_probe.json
.../jacobian/results/attach_probe.json | 48 +---------------------
1 file changed, 2 insertions(+), 46 deletions(-)

46 lines of measurement replaced by [{"model": "--help", "error": "missing"}]
by the command typed to find out what the script does. That file is the record of
which models genuinely attach the analytical Jacobian, which is what decides
membership in the suite's speedup table.

scriptwhat --help used to do
jacobian/probe_attach.pyprobed a model named --help; overwrote the committedresults/attach_probe.json
ode_engines_s4_sbml/check_sbml_engine_agreement.pyran bngsim + RoadRunner + COPASI over the Table S4 models, then overwrote its committed report
ode_fullnet/recover_s4_points.pyregenerated two networks through BNG2.pl, rewrote the characterization under parity_checks/, then copied both into another repository'slatex/generated/
ssa_table5/convert_all.pyreconverted all 14 models, rewrote results/converted/ + conversion_log.json
ssa_table5/emit_ssa_table.pyrewrote ssa_timing_ballpark.json in place and re-rendered the .md
ssa_table5/merge_jobout.pyrebuilt ssa_timing_ballpark.json from _jobout/
jacobian/diagnose_divergence.pyread --help as a model id and stopped — the one benign case

The fix

All seven build an ArgumentParser: --help exits 0 with usage, an unrecognized
flag exits 2, a bare invocation is unchanged.

Where a probe needs somewhere harmless to write, it now has a flag — the same
shape --out took on the PSA companion in #490:

  • --out on probe_attach.py and check_sbml_engine_agreement.py;
  • --paper-dir / --no-copy on recover_s4_points.py. That one also skips
    rather than crashes
    when the paper checkout is absent: the merge has already
    happened by then, so a FileNotFoundError at the copy left the work done and
    the run marked failed. Its stale /Users/wish/Code/PyBNF/... follow-up line is
    gone too, per your note that it was never a contract.

diagnose_divergence.py is both driver and worker — it re-enters itself as
--worker on an argv it composes in _spawn(). That branch is matched before
the parser and left byte-identical; only the half a person types is parsed.

The exemption, endorsed

ssa_table5/_ssa_cell.py (spawned per (engine, model) by run_ssa_timing.py,
Table 5's measurement path) and sbml_test_suite/testrunner/bngsim_wrapper.py
(invoked by the vendored runner on %d %n %o %l %v) get no parser. Each now
names its caller in a comment saying why there is no parser.

Worth noting: bngsim_wrapper.py was already correct. Its argc check answers
--help with a usage line and exit 2 — the issue's table overstated it as
ignoring argv. It needed the comment, not a change.

The endorsement is a test, not a convention:

deftest_spawned_workers_keep_their_positional_contract(rel):
src= (SUITES_DIR/rel).read_text()
assert"argparse"notinsrc, f"{rel} grew a parser -- it is spawned by ..."assertSPAWNED_WORKERS[rel] insrc, f"{rel} does not name its caller"

So the next argv sweep cannot quietly "fix" these two, and cannot read the
missing parser as an oversight either.

The test now discovers its family

test_benchmark_runner_help.py no longer derives from run_all.py's registry
plus a hand-kept companion list. It globs every suites/ script with a
__main__ guard
and subtracts SPAWNED_WORKERS — 59 scripts, 127 cases,
15 s. A script added to any suite inherits the contract without an edit here.
A separate test asserts every registry-named script lands inside that family, so
the orchestrator link stays explicit.

Two tests assert --help leaves each committed artifact byte-identical, which
states the actual harm rather than only "exit 0".

Verification

  • 127 cases pass; full local suite green.
  • Re-ran every script's --help with libsbml, scipy, roadrunner, amici,
    COPASI, antimony, gillespy2, diffrax, jax, pandas, matplotlib and
    psutil blocked at import. Only two scripts import an optional engine at module
    scope — biomodels/filter.py (libsbml) and showcase/run_ode_trf_fit_from_net.py
    (scipy) — and both deps are guaranteed present wherever this test runs
    (python-libsbml is a core dependency; scipy comes in via the test extra,
    which dev includes and CI syncs).
  • diagnose_divergence.py --worker verified to still reach worker() and fail
    inside it on a bogus path, proving the spawn argv unpack is intact.
  • The two committed artifacts are byte-identical in this branch.

Risk

None to any measured value. Every script behaves identically when invoked as it
was before, and the two scripts on a measurement path are the two that were not
touched functionally.

#488 fixed the two runners the orchestrator can reach. This finishes the
sweep over the seven scripts it cannot, and the finding is that the
hazard was demonstrated rather than argued:
* jacobian/probe_attach.py read `--help` as a model id, failed to find
a model by that name, and wrote the one-row failure over the
COMMITTED results/attach_probe.json -- 46 lines of measurement
replaced by two, by a command typed to find out what the script
does. Reproduced against the file in this repository.
* ode_engines_s4_sbml/check_sbml_engine_agreement.py ran bngsim,
RoadRunner and COPASI over the Table S4 models, then overwrote its
committed report.
* ode_fullnet/recover_s4_points.py reached furthest: regenerate two
networks through BNG2.pl, rewrite the characterization under
parity_checks/, then copy both files into a DIFFERENT repository's
latex/generated/, one of them committed there.
* ssa_table5/{convert_all,emit_ssa_table,merge_jobout}.py reconverted
the corpus or rewrote the Table 5 result set.
* jacobian/diagnose_divergence.py read `--help` as a model id (and
stopped there, the one benign case).
All seven now build an ArgumentParser: `--help` exits 0 with usage, an
unrecognized flag exits 2, a bare invocation is unchanged. Where a probe
needs somewhere harmless to write, it gets a flag -- `--out` on
probe_attach.py and check_sbml_engine_agreement.py, `--paper-dir` /
`--no-copy` on recover_s4_points.py, which also skips rather than
crashes when the paper checkout is absent. diagnose_divergence.py
re-enters itself as `--worker` on an argv it composes; that half is
matched before the parser and left exactly as it was.
Two scripts are exempt, at the paper side's explicit ask on the issue:
ssa_table5/_ssa_cell.py is spawned per (engine, model) by
run_ssa_timing.py, which is Table 5's measurement path, and
bngsim_wrapper.py is invoked by the vendored SBML test-suite runner on
the fixed `%d %n %o %l %v` template. A parser in either changes a
caller's contract to buy a usage line for an invocation nobody makes.
Each names its caller instead, and the wrapper already refused a hand
invocation with usage + exit 2.
test_benchmark_runner_help.py now discovers its family by structure --
every suites/ script with a `__main__` guard, minus that exemption list
-- rather than from run_all.py's registry plus a hand-kept companion
list. 127 cases, 15 s. A script added to any suite inherits the contract
without an edit here, and the exemptions are themselves tested: no
parser, and the worker must still name its caller, so the omission reads
as intent. Two further tests assert `--help` leaves each committed
artifact byte-identical.
Verified with libsbml, scipy, roadrunner, amici, COPASI and antimony
blocked at import: only the two scripts that import an optional engine
at module scope are affected, and both of those deps (python-libsbml
core, scipy via the test extra) are present wherever this test runs.
No measured value changes.
CI was red on 19 cases, all in the new test file, all one defect it had
just been widened enough to see.
Eight scripts resolved their repo root as
BNGSIM = Path(os.environ.get("BNGSIM_ROOT", Path.home() / "Code" / "bngsim"))
and then put <root>/parity_checks and <root>/parity_checks/bng_parity on
sys.path. So `import _bng_common` at module scope resolved only on a
machine whose clone happens to sit at ~/Code/bngsim, and raised
ModuleNotFoundError in any other checkout -- a worktree, a fresh clone,
and every CI runner. `--help` on those eight could not answer because the
module could not import:
ode_engines_s3/run_s3_timing.py (and patch_amici_split.py, which
imports it)
ode_engines_s4_sbml/run_s4_timing.py
ode_engines_s4_sbml/check_sbml_engine_agreement.py
ode_fullnet/{run_timing,run_forced,gen_networks,probe_growth,
recover_s4_points}.py
The default is now Path(__file__).resolve().parents[3] -- the checkout the
file lives in -- which is the idiom biomodels/emit.py already uses
(`BNGSIM_ROOT = BENCH_DIR.parents[2]`). BNGSIM_ROOT still overrides, so
the case the env var was written for is untouched: a different venv run
against a canonical checkout. Where the two already coincided, the
resolved path is identical and nothing moves.
The test could not have caught this, because the author's $HOME made the
wrong default look right. Every probe now runs with HOME (and USERPROFILE)
pointed at an empty session-scoped directory, so "anyone can run --help"
means anyone. Verified by stashing one script's fix: the two cases for it
fail locally with the same ModuleNotFoundError CI reported.
Also verified with amici, COPASI, gillespy2, diffrax, basico, pybnf,
psutil, matplotlib and bioservices blocked at import and BNGPATH /
SBML_TEST_SUITE_DIR cleared, alongside the scrubbed HOME: all 59 scripts
answer --help.
@wshlavacek

Copy link
Copy Markdown
CollaboratorAuthor

CI was red — 19 cases, all in the new test file, all one defect it had just been widened enough to see. Fixed in c031852.

Eight scripts resolved their repo root as Path.home() / "Code" / "bngsim" and then put <root>/parity_checks on sys.path. Their module-scope import _bng_common therefore resolved only on a machine whose clone sits at that path, and raised ModuleNotFoundError in any other checkout — a worktree, a fresh clone, every CI runner:

ode_engines_s3/run_s3_timing.py (+ patch_amici_split.py, which imports it)
ode_engines_s4_sbml/run_s4_timing.py
ode_engines_s4_sbml/check_sbml_engine_agreement.py
ode_fullnet/{run_timing,run_forced,gen_networks,probe_growth,recover_s4_points}.py

The default is now Path(__file__).resolve().parents[3] — the checkout the file lives in — which is the idiom biomodels/emit.py already uses. BNGSIM_ROOT still overrides, so the case the env var was written for (a different venv run against a canonical checkout) is untouched, and where the two already coincided the resolved path is identical.

The test could not have caught it, because the author's $HOME made the wrong default look right. Every probe now runs with HOME/USERPROFILE pointed at an empty session-scoped directory. Stashing one script's fix reproduces the exact CI failure locally, so this class is now caught on a dev machine rather than on a runner.

Worth noting this is the same shape as the issue itself: a script that only works where its author's files happen to be. It just showed up in sys.path instead of in an output path.

@wshlavacek
wshlavacek merged commit 89c2a5c into mainAug 27, 2026
4 checks passed
@wshlavacek
wshlavacek deleted the bench-argv-entry-points-489 branch August 27, 2026 16:46
Sign up for freeto 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.

benchmarks/suites: nine scripts outside run_all.py's registry still ignore argv

1 participant

@wshlavacek
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

benchmarks: settle the argv contract for the whole suites/ tree (#489) - #491

Merged
wshlavacek merged 2 commits into
mainfrom
bench-argv-entry-points-489
Aug 27, 2026
Merged

benchmarks: settle the argv contract for the whole suites/ tree (#489)#491
wshlavacek merged 2 commits into
mainfrom
bench-argv-entry-points-489

Conversation

@wshlavacek

Copy link
Copy Markdown
Collaborator

Closes#489. Follows #490, which fixed
the two runners run_all.py can reach.

The paper side's comment on the issue is followed exactly: _ssa_cell.py and
bngsim_wrapper.py get no parser, that exemption is now endorsed by a test
rather than left to the next implementer, and the freedom granted over
recover_s4_points.py is used.

The hazard was demonstrated, not argued

Two of the seven wrote committed files on a bare invocation. Reproduced
against this repository, then restored from git:

$ cd benchmarks/suites/jacobian && python probe_attach.py --help
--help: attached=None n_func_rxns=None ns=None self_check_failed=None err=missing
WROTE results/attach_probe.json
$ git diff --stat -- results/attach_probe.json
.../jacobian/results/attach_probe.json | 48 +---------------------
1 file changed, 2 insertions(+), 46 deletions(-)

46 lines of measurement replaced by [{"model": "--help", "error": "missing"}]
by the command typed to find out what the script does. That file is the record of
which models genuinely attach the analytical Jacobian, which is what decides
membership in the suite's speedup table.

scriptwhat --help used to do
jacobian/probe_attach.pyprobed a model named --help; overwrote the committedresults/attach_probe.json
ode_engines_s4_sbml/check_sbml_engine_agreement.pyran bngsim + RoadRunner + COPASI over the Table S4 models, then overwrote its committed report
ode_fullnet/recover_s4_points.pyregenerated two networks through BNG2.pl, rewrote the characterization under parity_checks/, then copied both into another repository'slatex/generated/
ssa_table5/convert_all.pyreconverted all 14 models, rewrote results/converted/ + conversion_log.json
ssa_table5/emit_ssa_table.pyrewrote ssa_timing_ballpark.json in place and re-rendered the .md
ssa_table5/merge_jobout.pyrebuilt ssa_timing_ballpark.json from _jobout/
jacobian/diagnose_divergence.pyread --help as a model id and stopped — the one benign case

The fix

All seven build an ArgumentParser: --help exits 0 with usage, an unrecognized
flag exits 2, a bare invocation is unchanged.

Where a probe needs somewhere harmless to write, it now has a flag — the same
shape --out took on the PSA companion in #490:

  • --out on probe_attach.py and check_sbml_engine_agreement.py;
  • --paper-dir / --no-copy on recover_s4_points.py. That one also skips
    rather than crashes
    when the paper checkout is absent: the merge has already
    happened by then, so a FileNotFoundError at the copy left the work done and
    the run marked failed. Its stale /Users/wish/Code/PyBNF/... follow-up line is
    gone too, per your note that it was never a contract.

diagnose_divergence.py is both driver and worker — it re-enters itself as
--worker on an argv it composes in _spawn(). That branch is matched before
the parser and left byte-identical; only the half a person types is parsed.

The exemption, endorsed

ssa_table5/_ssa_cell.py (spawned per (engine, model) by run_ssa_timing.py,
Table 5's measurement path) and sbml_test_suite/testrunner/bngsim_wrapper.py
(invoked by the vendored runner on %d %n %o %l %v) get no parser. Each now
names its caller in a comment saying why there is no parser.

Worth noting: bngsim_wrapper.py was already correct. Its argc check answers
--help with a usage line and exit 2 — the issue's table overstated it as
ignoring argv. It needed the comment, not a change.

The endorsement is a test, not a convention:

deftest_spawned_workers_keep_their_positional_contract(rel):
src= (SUITES_DIR/rel).read_text()
assert"argparse"notinsrc, f"{rel} grew a parser -- it is spawned by ..."assertSPAWNED_WORKERS[rel] insrc, f"{rel} does not name its caller"

So the next argv sweep cannot quietly "fix" these two, and cannot read the
missing parser as an oversight either.

The test now discovers its family

test_benchmark_runner_help.py no longer derives from run_all.py's registry
plus a hand-kept companion list. It globs every suites/ script with a
__main__ guard
and subtracts SPAWNED_WORKERS — 59 scripts, 127 cases,
15 s. A script added to any suite inherits the contract without an edit here.
A separate test asserts every registry-named script lands inside that family, so
the orchestrator link stays explicit.

Two tests assert --help leaves each committed artifact byte-identical, which
states the actual harm rather than only "exit 0".

Verification

  • 127 cases pass; full local suite green.
  • Re-ran every script's --help with libsbml, scipy, roadrunner, amici,
    COPASI, antimony, gillespy2, diffrax, jax, pandas, matplotlib and
    psutil blocked at import. Only two scripts import an optional engine at module
    scope — biomodels/filter.py (libsbml) and showcase/run_ode_trf_fit_from_net.py
    (scipy) — and both deps are guaranteed present wherever this test runs
    (python-libsbml is a core dependency; scipy comes in via the test extra,
    which dev includes and CI syncs).
  • diagnose_divergence.py --worker verified to still reach worker() and fail
    inside it on a bogus path, proving the spawn argv unpack is intact.
  • The two committed artifacts are byte-identical in this branch.

Risk

None to any measured value. Every script behaves identically when invoked as it
was before, and the two scripts on a measurement path are the two that were not
touched functionally.

#488 fixed the two runners the orchestrator can reach. This finishes the
sweep over the seven scripts it cannot, and the finding is that the
hazard was demonstrated rather than argued:
* jacobian/probe_attach.py read `--help` as a model id, failed to find
a model by that name, and wrote the one-row failure over the
COMMITTED results/attach_probe.json -- 46 lines of measurement
replaced by two, by a command typed to find out what the script
does. Reproduced against the file in this repository.
* ode_engines_s4_sbml/check_sbml_engine_agreement.py ran bngsim,
RoadRunner and COPASI over the Table S4 models, then overwrote its
committed report.
* ode_fullnet/recover_s4_points.py reached furthest: regenerate two
networks through BNG2.pl, rewrite the characterization under
parity_checks/, then copy both files into a DIFFERENT repository's
latex/generated/, one of them committed there.
* ssa_table5/{convert_all,emit_ssa_table,merge_jobout}.py reconverted
the corpus or rewrote the Table 5 result set.
* jacobian/diagnose_divergence.py read `--help` as a model id (and
stopped there, the one benign case).
All seven now build an ArgumentParser: `--help` exits 0 with usage, an
unrecognized flag exits 2, a bare invocation is unchanged. Where a probe
needs somewhere harmless to write, it gets a flag -- `--out` on
probe_attach.py and check_sbml_engine_agreement.py, `--paper-dir` /
`--no-copy` on recover_s4_points.py, which also skips rather than
crashes when the paper checkout is absent. diagnose_divergence.py
re-enters itself as `--worker` on an argv it composes; that half is
matched before the parser and left exactly as it was.
Two scripts are exempt, at the paper side's explicit ask on the issue:
ssa_table5/_ssa_cell.py is spawned per (engine, model) by
run_ssa_timing.py, which is Table 5's measurement path, and
bngsim_wrapper.py is invoked by the vendored SBML test-suite runner on
the fixed `%d %n %o %l %v` template. A parser in either changes a
caller's contract to buy a usage line for an invocation nobody makes.
Each names its caller instead, and the wrapper already refused a hand
invocation with usage + exit 2.
test_benchmark_runner_help.py now discovers its family by structure --
every suites/ script with a `__main__` guard, minus that exemption list
-- rather than from run_all.py's registry plus a hand-kept companion
list. 127 cases, 15 s. A script added to any suite inherits the contract
without an edit here, and the exemptions are themselves tested: no
parser, and the worker must still name its caller, so the omission reads
as intent. Two further tests assert `--help` leaves each committed
artifact byte-identical.
Verified with libsbml, scipy, roadrunner, amici, COPASI and antimony
blocked at import: only the two scripts that import an optional engine
at module scope are affected, and both of those deps (python-libsbml
core, scipy via the test extra) are present wherever this test runs.
No measured value changes.
CI was red on 19 cases, all in the new test file, all one defect it had
just been widened enough to see.
Eight scripts resolved their repo root as
BNGSIM = Path(os.environ.get("BNGSIM_ROOT", Path.home() / "Code" / "bngsim"))
and then put <root>/parity_checks and <root>/parity_checks/bng_parity on
sys.path. So `import _bng_common` at module scope resolved only on a
machine whose clone happens to sit at ~/Code/bngsim, and raised
ModuleNotFoundError in any other checkout -- a worktree, a fresh clone,
and every CI runner. `--help` on those eight could not answer because the
module could not import:
ode_engines_s3/run_s3_timing.py (and patch_amici_split.py, which
imports it)
ode_engines_s4_sbml/run_s4_timing.py
ode_engines_s4_sbml/check_sbml_engine_agreement.py
ode_fullnet/{run_timing,run_forced,gen_networks,probe_growth,
recover_s4_points}.py
The default is now Path(__file__).resolve().parents[3] -- the checkout the
file lives in -- which is the idiom biomodels/emit.py already uses
(`BNGSIM_ROOT = BENCH_DIR.parents[2]`). BNGSIM_ROOT still overrides, so
the case the env var was written for is untouched: a different venv run
against a canonical checkout. Where the two already coincided, the
resolved path is identical and nothing moves.
The test could not have caught this, because the author's $HOME made the
wrong default look right. Every probe now runs with HOME (and USERPROFILE)
pointed at an empty session-scoped directory, so "anyone can run --help"
means anyone. Verified by stashing one script's fix: the two cases for it
fail locally with the same ModuleNotFoundError CI reported.
Also verified with amici, COPASI, gillespy2, diffrax, basico, pybnf,
psutil, matplotlib and bioservices blocked at import and BNGPATH /
SBML_TEST_SUITE_DIR cleared, alongside the scrubbed HOME: all 59 scripts
answer --help.
@wshlavacek

Copy link
Copy Markdown
CollaboratorAuthor

CI was red — 19 cases, all in the new test file, all one defect it had just been widened enough to see. Fixed in c031852.

Eight scripts resolved their repo root as Path.home() / "Code" / "bngsim" and then put <root>/parity_checks on sys.path. Their module-scope import _bng_common therefore resolved only on a machine whose clone sits at that path, and raised ModuleNotFoundError in any other checkout — a worktree, a fresh clone, every CI runner:

ode_engines_s3/run_s3_timing.py (+ patch_amici_split.py, which imports it)
ode_engines_s4_sbml/run_s4_timing.py
ode_engines_s4_sbml/check_sbml_engine_agreement.py
ode_fullnet/{run_timing,run_forced,gen_networks,probe_growth,recover_s4_points}.py

The default is now Path(__file__).resolve().parents[3] — the checkout the file lives in — which is the idiom biomodels/emit.py already uses. BNGSIM_ROOT still overrides, so the case the env var was written for (a different venv run against a canonical checkout) is untouched, and where the two already coincided the resolved path is identical.

The test could not have caught it, because the author's $HOME made the wrong default look right. Every probe now runs with HOME/USERPROFILE pointed at an empty session-scoped directory. Stashing one script's fix reproduces the exact CI failure locally, so this class is now caught on a dev machine rather than on a runner.

Worth noting this is the same shape as the issue itself: a script that only works where its author's files happen to be. It just showed up in sys.path instead of in an output path.

@wshlavacek
wshlavacek merged commit 89c2a5c into mainAug 27, 2026
4 checks passed
@wshlavacek
wshlavacek deleted the bench-argv-entry-points-489 branch August 27, 2026 16:46
Sign up for freeto 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.

benchmarks/suites: nine scripts outside run_all.py's registry still ignore argv

1 participant

@wshlavacek
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

benchmarks: settle the argv contract for the whole suites/ tree (#489) - #491

Merged
wshlavacek merged 2 commits into
mainfrom
bench-argv-entry-points-489
Aug 27, 2026
Merged

benchmarks: settle the argv contract for the whole suites/ tree (#489)#491
wshlavacek merged 2 commits into
mainfrom
bench-argv-entry-points-489

Conversation

@wshlavacek

Copy link
Copy Markdown
Collaborator

Closes#489. Follows #490, which fixed
the two runners run_all.py can reach.

The paper side's comment on the issue is followed exactly: _ssa_cell.py and
bngsim_wrapper.py get no parser, that exemption is now endorsed by a test
rather than left to the next implementer, and the freedom granted over
recover_s4_points.py is used.

The hazard was demonstrated, not argued

Two of the seven wrote committed files on a bare invocation. Reproduced
against this repository, then restored from git:

$ cd benchmarks/suites/jacobian && python probe_attach.py --help
--help: attached=None n_func_rxns=None ns=None self_check_failed=None err=missing
WROTE results/attach_probe.json
$ git diff --stat -- results/attach_probe.json
.../jacobian/results/attach_probe.json | 48 +---------------------
1 file changed, 2 insertions(+), 46 deletions(-)

46 lines of measurement replaced by [{"model": "--help", "error": "missing"}]
by the command typed to find out what the script does. That file is the record of
which models genuinely attach the analytical Jacobian, which is what decides
membership in the suite's speedup table.

scriptwhat --help used to do
jacobian/probe_attach.pyprobed a model named --help; overwrote the committedresults/attach_probe.json
ode_engines_s4_sbml/check_sbml_engine_agreement.pyran bngsim + RoadRunner + COPASI over the Table S4 models, then overwrote its committed report
ode_fullnet/recover_s4_points.pyregenerated two networks through BNG2.pl, rewrote the characterization under parity_checks/, then copied both into another repository'slatex/generated/
ssa_table5/convert_all.pyreconverted all 14 models, rewrote results/converted/ + conversion_log.json
ssa_table5/emit_ssa_table.pyrewrote ssa_timing_ballpark.json in place and re-rendered the .md
ssa_table5/merge_jobout.pyrebuilt ssa_timing_ballpark.json from _jobout/
jacobian/diagnose_divergence.pyread --help as a model id and stopped — the one benign case

The fix

All seven build an ArgumentParser: --help exits 0 with usage, an unrecognized
flag exits 2, a bare invocation is unchanged.

Where a probe needs somewhere harmless to write, it now has a flag — the same
shape --out took on the PSA companion in #490:

  • --out on probe_attach.py and check_sbml_engine_agreement.py;
  • --paper-dir / --no-copy on recover_s4_points.py. That one also skips
    rather than crashes
    when the paper checkout is absent: the merge has already
    happened by then, so a FileNotFoundError at the copy left the work done and
    the run marked failed. Its stale /Users/wish/Code/PyBNF/... follow-up line is
    gone too, per your note that it was never a contract.

diagnose_divergence.py is both driver and worker — it re-enters itself as
--worker on an argv it composes in _spawn(). That branch is matched before
the parser and left byte-identical; only the half a person types is parsed.

The exemption, endorsed

ssa_table5/_ssa_cell.py (spawned per (engine, model) by run_ssa_timing.py,
Table 5's measurement path) and sbml_test_suite/testrunner/bngsim_wrapper.py
(invoked by the vendored runner on %d %n %o %l %v) get no parser. Each now
names its caller in a comment saying why there is no parser.

Worth noting: bngsim_wrapper.py was already correct. Its argc check answers
--help with a usage line and exit 2 — the issue's table overstated it as
ignoring argv. It needed the comment, not a change.

The endorsement is a test, not a convention:

deftest_spawned_workers_keep_their_positional_contract(rel):
src= (SUITES_DIR/rel).read_text()
assert"argparse"notinsrc, f"{rel} grew a parser -- it is spawned by ..."assertSPAWNED_WORKERS[rel] insrc, f"{rel} does not name its caller"

So the next argv sweep cannot quietly "fix" these two, and cannot read the
missing parser as an oversight either.

The test now discovers its family

test_benchmark_runner_help.py no longer derives from run_all.py's registry
plus a hand-kept companion list. It globs every suites/ script with a
__main__ guard
and subtracts SPAWNED_WORKERS — 59 scripts, 127 cases,
15 s. A script added to any suite inherits the contract without an edit here.
A separate test asserts every registry-named script lands inside that family, so
the orchestrator link stays explicit.

Two tests assert --help leaves each committed artifact byte-identical, which
states the actual harm rather than only "exit 0".

Verification

  • 127 cases pass; full local suite green.
  • Re-ran every script's --help with libsbml, scipy, roadrunner, amici,
    COPASI, antimony, gillespy2, diffrax, jax, pandas, matplotlib and
    psutil blocked at import. Only two scripts import an optional engine at module
    scope — biomodels/filter.py (libsbml) and showcase/run_ode_trf_fit_from_net.py
    (scipy) — and both deps are guaranteed present wherever this test runs
    (python-libsbml is a core dependency; scipy comes in via the test extra,
    which dev includes and CI syncs).
  • diagnose_divergence.py --worker verified to still reach worker() and fail
    inside it on a bogus path, proving the spawn argv unpack is intact.
  • The two committed artifacts are byte-identical in this branch.

Risk

None to any measured value. Every script behaves identically when invoked as it
was before, and the two scripts on a measurement path are the two that were not
touched functionally.

#488 fixed the two runners the orchestrator can reach. This finishes the
sweep over the seven scripts it cannot, and the finding is that the
hazard was demonstrated rather than argued:
* jacobian/probe_attach.py read `--help` as a model id, failed to find
a model by that name, and wrote the one-row failure over the
COMMITTED results/attach_probe.json -- 46 lines of measurement
replaced by two, by a command typed to find out what the script
does. Reproduced against the file in this repository.
* ode_engines_s4_sbml/check_sbml_engine_agreement.py ran bngsim,
RoadRunner and COPASI over the Table S4 models, then overwrote its
committed report.
* ode_fullnet/recover_s4_points.py reached furthest: regenerate two
networks through BNG2.pl, rewrite the characterization under
parity_checks/, then copy both files into a DIFFERENT repository's
latex/generated/, one of them committed there.
* ssa_table5/{convert_all,emit_ssa_table,merge_jobout}.py reconverted
the corpus or rewrote the Table 5 result set.
* jacobian/diagnose_divergence.py read `--help` as a model id (and
stopped there, the one benign case).
All seven now build an ArgumentParser: `--help` exits 0 with usage, an
unrecognized flag exits 2, a bare invocation is unchanged. Where a probe
needs somewhere harmless to write, it gets a flag -- `--out` on
probe_attach.py and check_sbml_engine_agreement.py, `--paper-dir` /
`--no-copy` on recover_s4_points.py, which also skips rather than
crashes when the paper checkout is absent. diagnose_divergence.py
re-enters itself as `--worker` on an argv it composes; that half is
matched before the parser and left exactly as it was.
Two scripts are exempt, at the paper side's explicit ask on the issue:
ssa_table5/_ssa_cell.py is spawned per (engine, model) by
run_ssa_timing.py, which is Table 5's measurement path, and
bngsim_wrapper.py is invoked by the vendored SBML test-suite runner on
the fixed `%d %n %o %l %v` template. A parser in either changes a
caller's contract to buy a usage line for an invocation nobody makes.
Each names its caller instead, and the wrapper already refused a hand
invocation with usage + exit 2.
test_benchmark_runner_help.py now discovers its family by structure --
every suites/ script with a `__main__` guard, minus that exemption list
-- rather than from run_all.py's registry plus a hand-kept companion
list. 127 cases, 15 s. A script added to any suite inherits the contract
without an edit here, and the exemptions are themselves tested: no
parser, and the worker must still name its caller, so the omission reads
as intent. Two further tests assert `--help` leaves each committed
artifact byte-identical.
Verified with libsbml, scipy, roadrunner, amici, COPASI and antimony
blocked at import: only the two scripts that import an optional engine
at module scope are affected, and both of those deps (python-libsbml
core, scipy via the test extra) are present wherever this test runs.
No measured value changes.
CI was red on 19 cases, all in the new test file, all one defect it had
just been widened enough to see.
Eight scripts resolved their repo root as
BNGSIM = Path(os.environ.get("BNGSIM_ROOT", Path.home() / "Code" / "bngsim"))
and then put <root>/parity_checks and <root>/parity_checks/bng_parity on
sys.path. So `import _bng_common` at module scope resolved only on a
machine whose clone happens to sit at ~/Code/bngsim, and raised
ModuleNotFoundError in any other checkout -- a worktree, a fresh clone,
and every CI runner. `--help` on those eight could not answer because the
module could not import:
ode_engines_s3/run_s3_timing.py (and patch_amici_split.py, which
imports it)
ode_engines_s4_sbml/run_s4_timing.py
ode_engines_s4_sbml/check_sbml_engine_agreement.py
ode_fullnet/{run_timing,run_forced,gen_networks,probe_growth,
recover_s4_points}.py
The default is now Path(__file__).resolve().parents[3] -- the checkout the
file lives in -- which is the idiom biomodels/emit.py already uses
(`BNGSIM_ROOT = BENCH_DIR.parents[2]`). BNGSIM_ROOT still overrides, so
the case the env var was written for is untouched: a different venv run
against a canonical checkout. Where the two already coincided, the
resolved path is identical and nothing moves.
The test could not have caught this, because the author's $HOME made the
wrong default look right. Every probe now runs with HOME (and USERPROFILE)
pointed at an empty session-scoped directory, so "anyone can run --help"
means anyone. Verified by stashing one script's fix: the two cases for it
fail locally with the same ModuleNotFoundError CI reported.
Also verified with amici, COPASI, gillespy2, diffrax, basico, pybnf,
psutil, matplotlib and bioservices blocked at import and BNGPATH /
SBML_TEST_SUITE_DIR cleared, alongside the scrubbed HOME: all 59 scripts
answer --help.
@wshlavacek

Copy link
Copy Markdown
CollaboratorAuthor

CI was red — 19 cases, all in the new test file, all one defect it had just been widened enough to see. Fixed in c031852.

Eight scripts resolved their repo root as Path.home() / "Code" / "bngsim" and then put <root>/parity_checks on sys.path. Their module-scope import _bng_common therefore resolved only on a machine whose clone sits at that path, and raised ModuleNotFoundError in any other checkout — a worktree, a fresh clone, every CI runner:

ode_engines_s3/run_s3_timing.py (+ patch_amici_split.py, which imports it)
ode_engines_s4_sbml/run_s4_timing.py
ode_engines_s4_sbml/check_sbml_engine_agreement.py
ode_fullnet/{run_timing,run_forced,gen_networks,probe_growth,recover_s4_points}.py

The default is now Path(__file__).resolve().parents[3] — the checkout the file lives in — which is the idiom biomodels/emit.py already uses. BNGSIM_ROOT still overrides, so the case the env var was written for (a different venv run against a canonical checkout) is untouched, and where the two already coincided the resolved path is identical.

The test could not have caught it, because the author's $HOME made the wrong default look right. Every probe now runs with HOME/USERPROFILE pointed at an empty session-scoped directory. Stashing one script's fix reproduces the exact CI failure locally, so this class is now caught on a dev machine rather than on a runner.

Worth noting this is the same shape as the issue itself: a script that only works where its author's files happen to be. It just showed up in sys.path instead of in an output path.

@wshlavacek
wshlavacek merged commit 89c2a5c into mainAug 27, 2026
4 checks passed
@wshlavacek
wshlavacek deleted the bench-argv-entry-points-489 branch August 27, 2026 16:46
Sign up for freeto 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.

benchmarks/suites: nine scripts outside run_all.py's registry still ignore argv

1 participant

@wshlavacek
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

benchmarks: settle the argv contract for the whole suites/ tree (#489) - #491

Merged
wshlavacek merged 2 commits into
mainfrom
bench-argv-entry-points-489
Aug 27, 2026
Merged

benchmarks: settle the argv contract for the whole suites/ tree (#489)#491
wshlavacek merged 2 commits into
mainfrom
bench-argv-entry-points-489

Conversation

@wshlavacek

Copy link
Copy Markdown
Collaborator

Closes#489. Follows #490, which fixed
the two runners run_all.py can reach.

The paper side's comment on the issue is followed exactly: _ssa_cell.py and
bngsim_wrapper.py get no parser, that exemption is now endorsed by a test
rather than left to the next implementer, and the freedom granted over
recover_s4_points.py is used.

The hazard was demonstrated, not argued

Two of the seven wrote committed files on a bare invocation. Reproduced
against this repository, then restored from git:

$ cd benchmarks/suites/jacobian && python probe_attach.py --help
--help: attached=None n_func_rxns=None ns=None self_check_failed=None err=missing
WROTE results/attach_probe.json
$ git diff --stat -- results/attach_probe.json
.../jacobian/results/attach_probe.json | 48 +---------------------
1 file changed, 2 insertions(+), 46 deletions(-)

46 lines of measurement replaced by [{"model": "--help", "error": "missing"}]
by the command typed to find out what the script does. That file is the record of
which models genuinely attach the analytical Jacobian, which is what decides
membership in the suite's speedup table.

scriptwhat --help used to do
jacobian/probe_attach.pyprobed a model named --help; overwrote the committedresults/attach_probe.json
ode_engines_s4_sbml/check_sbml_engine_agreement.pyran bngsim + RoadRunner + COPASI over the Table S4 models, then overwrote its committed report
ode_fullnet/recover_s4_points.pyregenerated two networks through BNG2.pl, rewrote the characterization under parity_checks/, then copied both into another repository'slatex/generated/
ssa_table5/convert_all.pyreconverted all 14 models, rewrote results/converted/ + conversion_log.json
ssa_table5/emit_ssa_table.pyrewrote ssa_timing_ballpark.json in place and re-rendered the .md
ssa_table5/merge_jobout.pyrebuilt ssa_timing_ballpark.json from _jobout/
jacobian/diagnose_divergence.pyread --help as a model id and stopped — the one benign case

The fix

All seven build an ArgumentParser: --help exits 0 with usage, an unrecognized
flag exits 2, a bare invocation is unchanged.

Where a probe needs somewhere harmless to write, it now has a flag — the same
shape --out took on the PSA companion in #490:

  • --out on probe_attach.py and check_sbml_engine_agreement.py;
  • --paper-dir / --no-copy on recover_s4_points.py. That one also skips
    rather than crashes
    when the paper checkout is absent: the merge has already
    happened by then, so a FileNotFoundError at the copy left the work done and
    the run marked failed. Its stale /Users/wish/Code/PyBNF/... follow-up line is
    gone too, per your note that it was never a contract.

diagnose_divergence.py is both driver and worker — it re-enters itself as
--worker on an argv it composes in _spawn(). That branch is matched before
the parser and left byte-identical; only the half a person types is parsed.

The exemption, endorsed

ssa_table5/_ssa_cell.py (spawned per (engine, model) by run_ssa_timing.py,
Table 5's measurement path) and sbml_test_suite/testrunner/bngsim_wrapper.py
(invoked by the vendored runner on %d %n %o %l %v) get no parser. Each now
names its caller in a comment saying why there is no parser.

Worth noting: bngsim_wrapper.py was already correct. Its argc check answers
--help with a usage line and exit 2 — the issue's table overstated it as
ignoring argv. It needed the comment, not a change.

The endorsement is a test, not a convention:

deftest_spawned_workers_keep_their_positional_contract(rel):
src= (SUITES_DIR/rel).read_text()
assert"argparse"notinsrc, f"{rel} grew a parser -- it is spawned by ..."assertSPAWNED_WORKERS[rel] insrc, f"{rel} does not name its caller"

So the next argv sweep cannot quietly "fix" these two, and cannot read the
missing parser as an oversight either.

The test now discovers its family

test_benchmark_runner_help.py no longer derives from run_all.py's registry
plus a hand-kept companion list. It globs every suites/ script with a
__main__ guard
and subtracts SPAWNED_WORKERS — 59 scripts, 127 cases,
15 s. A script added to any suite inherits the contract without an edit here.
A separate test asserts every registry-named script lands inside that family, so
the orchestrator link stays explicit.

Two tests assert --help leaves each committed artifact byte-identical, which
states the actual harm rather than only "exit 0".

Verification

  • 127 cases pass; full local suite green.
  • Re-ran every script's --help with libsbml, scipy, roadrunner, amici,
    COPASI, antimony, gillespy2, diffrax, jax, pandas, matplotlib and
    psutil blocked at import. Only two scripts import an optional engine at module
    scope — biomodels/filter.py (libsbml) and showcase/run_ode_trf_fit_from_net.py
    (scipy) — and both deps are guaranteed present wherever this test runs
    (python-libsbml is a core dependency; scipy comes in via the test extra,
    which dev includes and CI syncs).
  • diagnose_divergence.py --worker verified to still reach worker() and fail
    inside it on a bogus path, proving the spawn argv unpack is intact.
  • The two committed artifacts are byte-identical in this branch.

Risk

None to any measured value. Every script behaves identically when invoked as it
was before, and the two scripts on a measurement path are the two that were not
touched functionally.

#488 fixed the two runners the orchestrator can reach. This finishes the
sweep over the seven scripts it cannot, and the finding is that the
hazard was demonstrated rather than argued:
* jacobian/probe_attach.py read `--help` as a model id, failed to find
a model by that name, and wrote the one-row failure over the
COMMITTED results/attach_probe.json -- 46 lines of measurement
replaced by two, by a command typed to find out what the script
does. Reproduced against the file in this repository.
* ode_engines_s4_sbml/check_sbml_engine_agreement.py ran bngsim,
RoadRunner and COPASI over the Table S4 models, then overwrote its
committed report.
* ode_fullnet/recover_s4_points.py reached furthest: regenerate two
networks through BNG2.pl, rewrite the characterization under
parity_checks/, then copy both files into a DIFFERENT repository's
latex/generated/, one of them committed there.
* ssa_table5/{convert_all,emit_ssa_table,merge_jobout}.py reconverted
the corpus or rewrote the Table 5 result set.
* jacobian/diagnose_divergence.py read `--help` as a model id (and
stopped there, the one benign case).
All seven now build an ArgumentParser: `--help` exits 0 with usage, an
unrecognized flag exits 2, a bare invocation is unchanged. Where a probe
needs somewhere harmless to write, it gets a flag -- `--out` on
probe_attach.py and check_sbml_engine_agreement.py, `--paper-dir` /
`--no-copy` on recover_s4_points.py, which also skips rather than
crashes when the paper checkout is absent. diagnose_divergence.py
re-enters itself as `--worker` on an argv it composes; that half is
matched before the parser and left exactly as it was.
Two scripts are exempt, at the paper side's explicit ask on the issue:
ssa_table5/_ssa_cell.py is spawned per (engine, model) by
run_ssa_timing.py, which is Table 5's measurement path, and
bngsim_wrapper.py is invoked by the vendored SBML test-suite runner on
the fixed `%d %n %o %l %v` template. A parser in either changes a
caller's contract to buy a usage line for an invocation nobody makes.
Each names its caller instead, and the wrapper already refused a hand
invocation with usage + exit 2.
test_benchmark_runner_help.py now discovers its family by structure --
every suites/ script with a `__main__` guard, minus that exemption list
-- rather than from run_all.py's registry plus a hand-kept companion
list. 127 cases, 15 s. A script added to any suite inherits the contract
without an edit here, and the exemptions are themselves tested: no
parser, and the worker must still name its caller, so the omission reads
as intent. Two further tests assert `--help` leaves each committed
artifact byte-identical.
Verified with libsbml, scipy, roadrunner, amici, COPASI and antimony
blocked at import: only the two scripts that import an optional engine
at module scope are affected, and both of those deps (python-libsbml
core, scipy via the test extra) are present wherever this test runs.
No measured value changes.
CI was red on 19 cases, all in the new test file, all one defect it had
just been widened enough to see.
Eight scripts resolved their repo root as
BNGSIM = Path(os.environ.get("BNGSIM_ROOT", Path.home() / "Code" / "bngsim"))
and then put <root>/parity_checks and <root>/parity_checks/bng_parity on
sys.path. So `import _bng_common` at module scope resolved only on a
machine whose clone happens to sit at ~/Code/bngsim, and raised
ModuleNotFoundError in any other checkout -- a worktree, a fresh clone,
and every CI runner. `--help` on those eight could not answer because the
module could not import:
ode_engines_s3/run_s3_timing.py (and patch_amici_split.py, which
imports it)
ode_engines_s4_sbml/run_s4_timing.py
ode_engines_s4_sbml/check_sbml_engine_agreement.py
ode_fullnet/{run_timing,run_forced,gen_networks,probe_growth,
recover_s4_points}.py
The default is now Path(__file__).resolve().parents[3] -- the checkout the
file lives in -- which is the idiom biomodels/emit.py already uses
(`BNGSIM_ROOT = BENCH_DIR.parents[2]`). BNGSIM_ROOT still overrides, so
the case the env var was written for is untouched: a different venv run
against a canonical checkout. Where the two already coincided, the
resolved path is identical and nothing moves.
The test could not have caught this, because the author's $HOME made the
wrong default look right. Every probe now runs with HOME (and USERPROFILE)
pointed at an empty session-scoped directory, so "anyone can run --help"
means anyone. Verified by stashing one script's fix: the two cases for it
fail locally with the same ModuleNotFoundError CI reported.
Also verified with amici, COPASI, gillespy2, diffrax, basico, pybnf,
psutil, matplotlib and bioservices blocked at import and BNGPATH /
SBML_TEST_SUITE_DIR cleared, alongside the scrubbed HOME: all 59 scripts
answer --help.
@wshlavacek

Copy link
Copy Markdown
CollaboratorAuthor

CI was red — 19 cases, all in the new test file, all one defect it had just been widened enough to see. Fixed in c031852.

Eight scripts resolved their repo root as Path.home() / "Code" / "bngsim" and then put <root>/parity_checks on sys.path. Their module-scope import _bng_common therefore resolved only on a machine whose clone sits at that path, and raised ModuleNotFoundError in any other checkout — a worktree, a fresh clone, every CI runner:

ode_engines_s3/run_s3_timing.py (+ patch_amici_split.py, which imports it)
ode_engines_s4_sbml/run_s4_timing.py
ode_engines_s4_sbml/check_sbml_engine_agreement.py
ode_fullnet/{run_timing,run_forced,gen_networks,probe_growth,recover_s4_points}.py

The default is now Path(__file__).resolve().parents[3] — the checkout the file lives in — which is the idiom biomodels/emit.py already uses. BNGSIM_ROOT still overrides, so the case the env var was written for (a different venv run against a canonical checkout) is untouched, and where the two already coincided the resolved path is identical.

The test could not have caught it, because the author's $HOME made the wrong default look right. Every probe now runs with HOME/USERPROFILE pointed at an empty session-scoped directory. Stashing one script's fix reproduces the exact CI failure locally, so this class is now caught on a dev machine rather than on a runner.

Worth noting this is the same shape as the issue itself: a script that only works where its author's files happen to be. It just showed up in sys.path instead of in an output path.

@wshlavacek
wshlavacek merged commit 89c2a5c into mainAug 27, 2026
4 checks passed
@wshlavacek
wshlavacek deleted the bench-argv-entry-points-489 branch August 27, 2026 16:46
Sign up for freeto 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.

benchmarks/suites: nine scripts outside run_all.py's registry still ignore argv

1 participant

@wshlavacek
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

benchmarks: settle the argv contract for the whole suites/ tree (#489) - #491

Merged
wshlavacek merged 2 commits into
mainfrom
bench-argv-entry-points-489
Aug 27, 2026
Merged

benchmarks: settle the argv contract for the whole suites/ tree (#489)#491
wshlavacek merged 2 commits into
mainfrom
bench-argv-entry-points-489

Conversation

@wshlavacek

Copy link
Copy Markdown
Collaborator

Closes#489. Follows #490, which fixed
the two runners run_all.py can reach.

The paper side's comment on the issue is followed exactly: _ssa_cell.py and
bngsim_wrapper.py get no parser, that exemption is now endorsed by a test
rather than left to the next implementer, and the freedom granted over
recover_s4_points.py is used.

The hazard was demonstrated, not argued

Two of the seven wrote committed files on a bare invocation. Reproduced
against this repository, then restored from git:

$ cd benchmarks/suites/jacobian && python probe_attach.py --help
--help: attached=None n_func_rxns=None ns=None self_check_failed=None err=missing
WROTE results/attach_probe.json
$ git diff --stat -- results/attach_probe.json
.../jacobian/results/attach_probe.json | 48 +---------------------
1 file changed, 2 insertions(+), 46 deletions(-)

46 lines of measurement replaced by [{"model": "--help", "error": "missing"}]
by the command typed to find out what the script does. That file is the record of
which models genuinely attach the analytical Jacobian, which is what decides
membership in the suite's speedup table.

scriptwhat --help used to do
jacobian/probe_attach.pyprobed a model named --help; overwrote the committedresults/attach_probe.json
ode_engines_s4_sbml/check_sbml_engine_agreement.pyran bngsim + RoadRunner + COPASI over the Table S4 models, then overwrote its committed report
ode_fullnet/recover_s4_points.pyregenerated two networks through BNG2.pl, rewrote the characterization under parity_checks/, then copied both into another repository'slatex/generated/
ssa_table5/convert_all.pyreconverted all 14 models, rewrote results/converted/ + conversion_log.json
ssa_table5/emit_ssa_table.pyrewrote ssa_timing_ballpark.json in place and re-rendered the .md
ssa_table5/merge_jobout.pyrebuilt ssa_timing_ballpark.json from _jobout/
jacobian/diagnose_divergence.pyread --help as a model id and stopped — the one benign case

The fix

All seven build an ArgumentParser: --help exits 0 with usage, an unrecognized
flag exits 2, a bare invocation is unchanged.

Where a probe needs somewhere harmless to write, it now has a flag — the same
shape --out took on the PSA companion in #490:

  • --out on probe_attach.py and check_sbml_engine_agreement.py;
  • --paper-dir / --no-copy on recover_s4_points.py. That one also skips
    rather than crashes
    when the paper checkout is absent: the merge has already
    happened by then, so a FileNotFoundError at the copy left the work done and
    the run marked failed. Its stale /Users/wish/Code/PyBNF/... follow-up line is
    gone too, per your note that it was never a contract.

diagnose_divergence.py is both driver and worker — it re-enters itself as
--worker on an argv it composes in _spawn(). That branch is matched before
the parser and left byte-identical; only the half a person types is parsed.

The exemption, endorsed

ssa_table5/_ssa_cell.py (spawned per (engine, model) by run_ssa_timing.py,
Table 5's measurement path) and sbml_test_suite/testrunner/bngsim_wrapper.py
(invoked by the vendored runner on %d %n %o %l %v) get no parser. Each now
names its caller in a comment saying why there is no parser.

Worth noting: bngsim_wrapper.py was already correct. Its argc check answers
--help with a usage line and exit 2 — the issue's table overstated it as
ignoring argv. It needed the comment, not a change.

The endorsement is a test, not a convention:

deftest_spawned_workers_keep_their_positional_contract(rel):
src= (SUITES_DIR/rel).read_text()
assert"argparse"notinsrc, f"{rel} grew a parser -- it is spawned by ..."assertSPAWNED_WORKERS[rel] insrc, f"{rel} does not name its caller"

So the next argv sweep cannot quietly "fix" these two, and cannot read the
missing parser as an oversight either.

The test now discovers its family

test_benchmark_runner_help.py no longer derives from run_all.py's registry
plus a hand-kept companion list. It globs every suites/ script with a
__main__ guard
and subtracts SPAWNED_WORKERS — 59 scripts, 127 cases,
15 s. A script added to any suite inherits the contract without an edit here.
A separate test asserts every registry-named script lands inside that family, so
the orchestrator link stays explicit.

Two tests assert --help leaves each committed artifact byte-identical, which
states the actual harm rather than only "exit 0".

Verification

  • 127 cases pass; full local suite green.
  • Re-ran every script's --help with libsbml, scipy, roadrunner, amici,
    COPASI, antimony, gillespy2, diffrax, jax, pandas, matplotlib and
    psutil blocked at import. Only two scripts import an optional engine at module
    scope — biomodels/filter.py (libsbml) and showcase/run_ode_trf_fit_from_net.py
    (scipy) — and both deps are guaranteed present wherever this test runs
    (python-libsbml is a core dependency; scipy comes in via the test extra,
    which dev includes and CI syncs).
  • diagnose_divergence.py --worker verified to still reach worker() and fail
    inside it on a bogus path, proving the spawn argv unpack is intact.
  • The two committed artifacts are byte-identical in this branch.

Risk

None to any measured value. Every script behaves identically when invoked as it
was before, and the two scripts on a measurement path are the two that were not
touched functionally.

#488 fixed the two runners the orchestrator can reach. This finishes the
sweep over the seven scripts it cannot, and the finding is that the
hazard was demonstrated rather than argued:
* jacobian/probe_attach.py read `--help` as a model id, failed to find
a model by that name, and wrote the one-row failure over the
COMMITTED results/attach_probe.json -- 46 lines of measurement
replaced by two, by a command typed to find out what the script
does. Reproduced against the file in this repository.
* ode_engines_s4_sbml/check_sbml_engine_agreement.py ran bngsim,
RoadRunner and COPASI over the Table S4 models, then overwrote its
committed report.
* ode_fullnet/recover_s4_points.py reached furthest: regenerate two
networks through BNG2.pl, rewrite the characterization under
parity_checks/, then copy both files into a DIFFERENT repository's
latex/generated/, one of them committed there.
* ssa_table5/{convert_all,emit_ssa_table,merge_jobout}.py reconverted
the corpus or rewrote the Table 5 result set.
* jacobian/diagnose_divergence.py read `--help` as a model id (and
stopped there, the one benign case).
All seven now build an ArgumentParser: `--help` exits 0 with usage, an
unrecognized flag exits 2, a bare invocation is unchanged. Where a probe
needs somewhere harmless to write, it gets a flag -- `--out` on
probe_attach.py and check_sbml_engine_agreement.py, `--paper-dir` /
`--no-copy` on recover_s4_points.py, which also skips rather than
crashes when the paper checkout is absent. diagnose_divergence.py
re-enters itself as `--worker` on an argv it composes; that half is
matched before the parser and left exactly as it was.
Two scripts are exempt, at the paper side's explicit ask on the issue:
ssa_table5/_ssa_cell.py is spawned per (engine, model) by
run_ssa_timing.py, which is Table 5's measurement path, and
bngsim_wrapper.py is invoked by the vendored SBML test-suite runner on
the fixed `%d %n %o %l %v` template. A parser in either changes a
caller's contract to buy a usage line for an invocation nobody makes.
Each names its caller instead, and the wrapper already refused a hand
invocation with usage + exit 2.
test_benchmark_runner_help.py now discovers its family by structure --
every suites/ script with a `__main__` guard, minus that exemption list
-- rather than from run_all.py's registry plus a hand-kept companion
list. 127 cases, 15 s. A script added to any suite inherits the contract
without an edit here, and the exemptions are themselves tested: no
parser, and the worker must still name its caller, so the omission reads
as intent. Two further tests assert `--help` leaves each committed
artifact byte-identical.
Verified with libsbml, scipy, roadrunner, amici, COPASI and antimony
blocked at import: only the two scripts that import an optional engine
at module scope are affected, and both of those deps (python-libsbml
core, scipy via the test extra) are present wherever this test runs.
No measured value changes.
CI was red on 19 cases, all in the new test file, all one defect it had
just been widened enough to see.
Eight scripts resolved their repo root as
BNGSIM = Path(os.environ.get("BNGSIM_ROOT", Path.home() / "Code" / "bngsim"))
and then put <root>/parity_checks and <root>/parity_checks/bng_parity on
sys.path. So `import _bng_common` at module scope resolved only on a
machine whose clone happens to sit at ~/Code/bngsim, and raised
ModuleNotFoundError in any other checkout -- a worktree, a fresh clone,
and every CI runner. `--help` on those eight could not answer because the
module could not import:
ode_engines_s3/run_s3_timing.py (and patch_amici_split.py, which
imports it)
ode_engines_s4_sbml/run_s4_timing.py
ode_engines_s4_sbml/check_sbml_engine_agreement.py
ode_fullnet/{run_timing,run_forced,gen_networks,probe_growth,
recover_s4_points}.py
The default is now Path(__file__).resolve().parents[3] -- the checkout the
file lives in -- which is the idiom biomodels/emit.py already uses
(`BNGSIM_ROOT = BENCH_DIR.parents[2]`). BNGSIM_ROOT still overrides, so
the case the env var was written for is untouched: a different venv run
against a canonical checkout. Where the two already coincided, the
resolved path is identical and nothing moves.
The test could not have caught this, because the author's $HOME made the
wrong default look right. Every probe now runs with HOME (and USERPROFILE)
pointed at an empty session-scoped directory, so "anyone can run --help"
means anyone. Verified by stashing one script's fix: the two cases for it
fail locally with the same ModuleNotFoundError CI reported.
Also verified with amici, COPASI, gillespy2, diffrax, basico, pybnf,
psutil, matplotlib and bioservices blocked at import and BNGPATH /
SBML_TEST_SUITE_DIR cleared, alongside the scrubbed HOME: all 59 scripts
answer --help.
@wshlavacek

Copy link
Copy Markdown
CollaboratorAuthor

CI was red — 19 cases, all in the new test file, all one defect it had just been widened enough to see. Fixed in c031852.

Eight scripts resolved their repo root as Path.home() / "Code" / "bngsim" and then put <root>/parity_checks on sys.path. Their module-scope import _bng_common therefore resolved only on a machine whose clone sits at that path, and raised ModuleNotFoundError in any other checkout — a worktree, a fresh clone, every CI runner:

ode_engines_s3/run_s3_timing.py (+ patch_amici_split.py, which imports it)
ode_engines_s4_sbml/run_s4_timing.py
ode_engines_s4_sbml/check_sbml_engine_agreement.py
ode_fullnet/{run_timing,run_forced,gen_networks,probe_growth,recover_s4_points}.py

The default is now Path(__file__).resolve().parents[3] — the checkout the file lives in — which is the idiom biomodels/emit.py already uses. BNGSIM_ROOT still overrides, so the case the env var was written for (a different venv run against a canonical checkout) is untouched, and where the two already coincided the resolved path is identical.

The test could not have caught it, because the author's $HOME made the wrong default look right. Every probe now runs with HOME/USERPROFILE pointed at an empty session-scoped directory. Stashing one script's fix reproduces the exact CI failure locally, so this class is now caught on a dev machine rather than on a runner.

Worth noting this is the same shape as the issue itself: a script that only works where its author's files happen to be. It just showed up in sys.path instead of in an output path.

@wshlavacek
wshlavacek merged commit 89c2a5c into mainAug 27, 2026
4 checks passed
@wshlavacek
wshlavacek deleted the bench-argv-entry-points-489 branch August 27, 2026 16:46
Sign up for freeto 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.

benchmarks/suites: nine scripts outside run_all.py's registry still ignore argv

1 participant

@wshlavacek
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

benchmarks: settle the argv contract for the whole suites/ tree (#489) - #491

Merged
wshlavacek merged 2 commits into
mainfrom
bench-argv-entry-points-489
Aug 27, 2026
Merged

benchmarks: settle the argv contract for the whole suites/ tree (#489)#491
wshlavacek merged 2 commits into
mainfrom
bench-argv-entry-points-489

Conversation

@wshlavacek

Copy link
Copy Markdown
Collaborator

Closes#489. Follows #490, which fixed
the two runners run_all.py can reach.

The paper side's comment on the issue is followed exactly: _ssa_cell.py and
bngsim_wrapper.py get no parser, that exemption is now endorsed by a test
rather than left to the next implementer, and the freedom granted over
recover_s4_points.py is used.

The hazard was demonstrated, not argued

Two of the seven wrote committed files on a bare invocation. Reproduced
against this repository, then restored from git:

$ cd benchmarks/suites/jacobian && python probe_attach.py --help
--help: attached=None n_func_rxns=None ns=None self_check_failed=None err=missing
WROTE results/attach_probe.json
$ git diff --stat -- results/attach_probe.json
.../jacobian/results/attach_probe.json | 48 +---------------------
1 file changed, 2 insertions(+), 46 deletions(-)

46 lines of measurement replaced by [{"model": "--help", "error": "missing"}]
by the command typed to find out what the script does. That file is the record of
which models genuinely attach the analytical Jacobian, which is what decides
membership in the suite's speedup table.

scriptwhat --help used to do
jacobian/probe_attach.pyprobed a model named --help; overwrote the committedresults/attach_probe.json
ode_engines_s4_sbml/check_sbml_engine_agreement.pyran bngsim + RoadRunner + COPASI over the Table S4 models, then overwrote its committed report
ode_fullnet/recover_s4_points.pyregenerated two networks through BNG2.pl, rewrote the characterization under parity_checks/, then copied both into another repository'slatex/generated/
ssa_table5/convert_all.pyreconverted all 14 models, rewrote results/converted/ + conversion_log.json
ssa_table5/emit_ssa_table.pyrewrote ssa_timing_ballpark.json in place and re-rendered the .md
ssa_table5/merge_jobout.pyrebuilt ssa_timing_ballpark.json from _jobout/
jacobian/diagnose_divergence.pyread --help as a model id and stopped — the one benign case

The fix

All seven build an ArgumentParser: --help exits 0 with usage, an unrecognized
flag exits 2, a bare invocation is unchanged.

Where a probe needs somewhere harmless to write, it now has a flag — the same
shape --out took on the PSA companion in #490:

  • --out on probe_attach.py and check_sbml_engine_agreement.py;
  • --paper-dir / --no-copy on recover_s4_points.py. That one also skips
    rather than crashes
    when the paper checkout is absent: the merge has already
    happened by then, so a FileNotFoundError at the copy left the work done and
    the run marked failed. Its stale /Users/wish/Code/PyBNF/... follow-up line is
    gone too, per your note that it was never a contract.

diagnose_divergence.py is both driver and worker — it re-enters itself as
--worker on an argv it composes in _spawn(). That branch is matched before
the parser and left byte-identical; only the half a person types is parsed.

The exemption, endorsed

ssa_table5/_ssa_cell.py (spawned per (engine, model) by run_ssa_timing.py,
Table 5's measurement path) and sbml_test_suite/testrunner/bngsim_wrapper.py
(invoked by the vendored runner on %d %n %o %l %v) get no parser. Each now
names its caller in a comment saying why there is no parser.

Worth noting: bngsim_wrapper.py was already correct. Its argc check answers
--help with a usage line and exit 2 — the issue's table overstated it as
ignoring argv. It needed the comment, not a change.

The endorsement is a test, not a convention:

deftest_spawned_workers_keep_their_positional_contract(rel):
src= (SUITES_DIR/rel).read_text()
assert"argparse"notinsrc, f"{rel} grew a parser -- it is spawned by ..."assertSPAWNED_WORKERS[rel] insrc, f"{rel} does not name its caller"

So the next argv sweep cannot quietly "fix" these two, and cannot read the
missing parser as an oversight either.

The test now discovers its family

test_benchmark_runner_help.py no longer derives from run_all.py's registry
plus a hand-kept companion list. It globs every suites/ script with a
__main__ guard
and subtracts SPAWNED_WORKERS — 59 scripts, 127 cases,
15 s. A script added to any suite inherits the contract without an edit here.
A separate test asserts every registry-named script lands inside that family, so
the orchestrator link stays explicit.

Two tests assert --help leaves each committed artifact byte-identical, which
states the actual harm rather than only "exit 0".

Verification

  • 127 cases pass; full local suite green.
  • Re-ran every script's --help with libsbml, scipy, roadrunner, amici,
    COPASI, antimony, gillespy2, diffrax, jax, pandas, matplotlib and
    psutil blocked at import. Only two scripts import an optional engine at module
    scope — biomodels/filter.py (libsbml) and showcase/run_ode_trf_fit_from_net.py
    (scipy) — and both deps are guaranteed present wherever this test runs
    (python-libsbml is a core dependency; scipy comes in via the test extra,
    which dev includes and CI syncs).
  • diagnose_divergence.py --worker verified to still reach worker() and fail
    inside it on a bogus path, proving the spawn argv unpack is intact.
  • The two committed artifacts are byte-identical in this branch.

Risk

None to any measured value. Every script behaves identically when invoked as it
was before, and the two scripts on a measurement path are the two that were not
touched functionally.

#488 fixed the two runners the orchestrator can reach. This finishes the
sweep over the seven scripts it cannot, and the finding is that the
hazard was demonstrated rather than argued:
* jacobian/probe_attach.py read `--help` as a model id, failed to find
a model by that name, and wrote the one-row failure over the
COMMITTED results/attach_probe.json -- 46 lines of measurement
replaced by two, by a command typed to find out what the script
does. Reproduced against the file in this repository.
* ode_engines_s4_sbml/check_sbml_engine_agreement.py ran bngsim,
RoadRunner and COPASI over the Table S4 models, then overwrote its
committed report.
* ode_fullnet/recover_s4_points.py reached furthest: regenerate two
networks through BNG2.pl, rewrite the characterization under
parity_checks/, then copy both files into a DIFFERENT repository's
latex/generated/, one of them committed there.
* ssa_table5/{convert_all,emit_ssa_table,merge_jobout}.py reconverted
the corpus or rewrote the Table 5 result set.
* jacobian/diagnose_divergence.py read `--help` as a model id (and
stopped there, the one benign case).
All seven now build an ArgumentParser: `--help` exits 0 with usage, an
unrecognized flag exits 2, a bare invocation is unchanged. Where a probe
needs somewhere harmless to write, it gets a flag -- `--out` on
probe_attach.py and check_sbml_engine_agreement.py, `--paper-dir` /
`--no-copy` on recover_s4_points.py, which also skips rather than
crashes when the paper checkout is absent. diagnose_divergence.py
re-enters itself as `--worker` on an argv it composes; that half is
matched before the parser and left exactly as it was.
Two scripts are exempt, at the paper side's explicit ask on the issue:
ssa_table5/_ssa_cell.py is spawned per (engine, model) by
run_ssa_timing.py, which is Table 5's measurement path, and
bngsim_wrapper.py is invoked by the vendored SBML test-suite runner on
the fixed `%d %n %o %l %v` template. A parser in either changes a
caller's contract to buy a usage line for an invocation nobody makes.
Each names its caller instead, and the wrapper already refused a hand
invocation with usage + exit 2.
test_benchmark_runner_help.py now discovers its family by structure --
every suites/ script with a `__main__` guard, minus that exemption list
-- rather than from run_all.py's registry plus a hand-kept companion
list. 127 cases, 15 s. A script added to any suite inherits the contract
without an edit here, and the exemptions are themselves tested: no
parser, and the worker must still name its caller, so the omission reads
as intent. Two further tests assert `--help` leaves each committed
artifact byte-identical.
Verified with libsbml, scipy, roadrunner, amici, COPASI and antimony
blocked at import: only the two scripts that import an optional engine
at module scope are affected, and both of those deps (python-libsbml
core, scipy via the test extra) are present wherever this test runs.
No measured value changes.
CI was red on 19 cases, all in the new test file, all one defect it had
just been widened enough to see.
Eight scripts resolved their repo root as
BNGSIM = Path(os.environ.get("BNGSIM_ROOT", Path.home() / "Code" / "bngsim"))
and then put <root>/parity_checks and <root>/parity_checks/bng_parity on
sys.path. So `import _bng_common` at module scope resolved only on a
machine whose clone happens to sit at ~/Code/bngsim, and raised
ModuleNotFoundError in any other checkout -- a worktree, a fresh clone,
and every CI runner. `--help` on those eight could not answer because the
module could not import:
ode_engines_s3/run_s3_timing.py (and patch_amici_split.py, which
imports it)
ode_engines_s4_sbml/run_s4_timing.py
ode_engines_s4_sbml/check_sbml_engine_agreement.py
ode_fullnet/{run_timing,run_forced,gen_networks,probe_growth,
recover_s4_points}.py
The default is now Path(__file__).resolve().parents[3] -- the checkout the
file lives in -- which is the idiom biomodels/emit.py already uses
(`BNGSIM_ROOT = BENCH_DIR.parents[2]`). BNGSIM_ROOT still overrides, so
the case the env var was written for is untouched: a different venv run
against a canonical checkout. Where the two already coincided, the
resolved path is identical and nothing moves.
The test could not have caught this, because the author's $HOME made the
wrong default look right. Every probe now runs with HOME (and USERPROFILE)
pointed at an empty session-scoped directory, so "anyone can run --help"
means anyone. Verified by stashing one script's fix: the two cases for it
fail locally with the same ModuleNotFoundError CI reported.
Also verified with amici, COPASI, gillespy2, diffrax, basico, pybnf,
psutil, matplotlib and bioservices blocked at import and BNGPATH /
SBML_TEST_SUITE_DIR cleared, alongside the scrubbed HOME: all 59 scripts
answer --help.
@wshlavacek

Copy link
Copy Markdown
CollaboratorAuthor

CI was red — 19 cases, all in the new test file, all one defect it had just been widened enough to see. Fixed in c031852.

Eight scripts resolved their repo root as Path.home() / "Code" / "bngsim" and then put <root>/parity_checks on sys.path. Their module-scope import _bng_common therefore resolved only on a machine whose clone sits at that path, and raised ModuleNotFoundError in any other checkout — a worktree, a fresh clone, every CI runner:

ode_engines_s3/run_s3_timing.py (+ patch_amici_split.py, which imports it)
ode_engines_s4_sbml/run_s4_timing.py
ode_engines_s4_sbml/check_sbml_engine_agreement.py
ode_fullnet/{run_timing,run_forced,gen_networks,probe_growth,recover_s4_points}.py

The default is now Path(__file__).resolve().parents[3] — the checkout the file lives in — which is the idiom biomodels/emit.py already uses. BNGSIM_ROOT still overrides, so the case the env var was written for (a different venv run against a canonical checkout) is untouched, and where the two already coincided the resolved path is identical.

The test could not have caught it, because the author's $HOME made the wrong default look right. Every probe now runs with HOME/USERPROFILE pointed at an empty session-scoped directory. Stashing one script's fix reproduces the exact CI failure locally, so this class is now caught on a dev machine rather than on a runner.

Worth noting this is the same shape as the issue itself: a script that only works where its author's files happen to be. It just showed up in sys.path instead of in an output path.

@wshlavacek
wshlavacek merged commit 89c2a5c into mainAug 27, 2026
4 checks passed
@wshlavacek
wshlavacek deleted the bench-argv-entry-points-489 branch August 27, 2026 16:46
Sign up for freeto 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.

benchmarks/suites: nine scripts outside run_all.py's registry still ignore argv

1 participant

@wshlavacek