Skip to content

Take the best of 15 timed pairs in the tapscript scaling ratio tests (issue #2613) - #2623

Merged
pmazzocchi merged 1 commit into
mainfrom
tapscript-scaling-2613
Oct 11, 2026
Merged

pmazzocchi merged 1 commit into
mainfrom
tapscript-scaling-2613

Conversation

@fametrano

Copy link
Copy Markdown
Member

This pull request cites ISS 2613 and does not close it. Nobody has measured the failure rate at a load of 20 or more, where the issue saw the ratio reach 12.0. The issue stays open for that measurement.

The two ratio tests in tests/script_engine/tapscript_scaling_test.py now share one helper, _ratio. It times 15 interleaved small and large runs instead of 5, and takes the best of each. The limit stays at 10, against 16 for a quadratic implementation. Load only adds time, and it is likelier to hit the longer run. A ratio of 12 from a quiet value of about 4 therefore needs every large run stretched. The best of 15 is far less likely to be stretched than the best of 5.

Measured at loads 2 to 11: the ratios were about 4, with extremes 2.3 and 6.0 and no failure. A reviewer planted a quadratic signature-check cost, and the test failed at 13.7. A CPU-time clock was rejected, because Windows' 15.6 ms granularity is coarser than the small run.

Gates on bd9e0ae, whose tree is this sha's (f2b5fa5 changes the message only), run by the writer under the gate lock:

  • uv run pytest: exit 0, coverage 100%
  • uv run pre-commit run --all-files: exit 0
  • sphinx-build -n -W -b html docs/source docs/build/html: exit 0

The local review cleared f2b5fa5, the branch's only commit on b71525a. No rebase was needed.

🤖 Generated with Claude Code

Comment thread tests/script_engine/tapscript_scaling_test.py

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ACK f2b5fa5

No blocking or non-blocking findings. One question is left inline, about the 300 s timeout now covering 15 timed pairs. It does not hold the ack.

@pmazzocchi pmazzocchi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A fresh local review cleared f2b5fa5. Nothing blocks it. Four points:

  1. Mutation cost. .github/mutation/engine.toml:31 and sig_hash.toml:43 run all of tests/script_engine for every mutant, under a 90-minute budget. On this head CI took 19.6 s and 21.3 s for the two ratio tests; 5-pair branches took 3.3–7.0 s. So each session reaches fewer mutants. One option is to deselect the ratio tests in those two test-commands. That is a design choice, so it is @fametrano's to make. This is an estimate from CI logs, not a measured session.
  2. Commit message: the 2.3–6.0 extremes are given "at load 5" there, "at loads 2 to 11" in the body, and a load of 13 in the thread. Make them agree in the squash.
  3. Commit message: "over 50 runs" is a count. It belongs in the body, not in what lands.
  4. Body: ISS 2613 is not a link.

@pmazzocchi

Copy link
Copy Markdown
Contributor

Claimed: a /btclib-pr session (pmazzocchi) is rebasing this onto main (bbb0555), running the gates and taking it to the decision. The branch holds no other change.

…(issue #2613)

The ratio of the best large run to the best small run failed at 12.0
against a limit of 10 at a 1-minute load of 20 to 23, twice, and passed
on a re-run. Interleaved timing (PR #2560) was already in place.

On a quiet machine the ratio is about 4, from 2.3 to 6.0 over 50 runs of
each case at load 5, and none failed. Load adds the time a run is
descheduled, which a longer run is more likely to suffer, so the best of
more runs is what is asked of both sides: 15 instead of 5, in one helper
for the two tests. The limit stays at 10 against 16 for a quadratic
implementation. The failure rate at a load of 20 or more is unmeasured, so
the issue stays open for it.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Ferdinando Ametrano <ferdinando@ametrano.net>
@pmazzocchi
pmazzocchi force-pushed the tapscript-scaling-2613 branch from f2b5fa5 to 42cc95d Compare October 11, 2026 21:30
Comment thread tests/script_engine/tapscript_scaling_test.py

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ACK 42cc95d

  • non-blocking, tests/script_engine/tapscript_scaling_test.py:151,161: the ratio tests now make 15 timed pairs instead of 5, but their timeout is still _BOUND * 5 (300 s). That leaves about 16 s for a linear large run, down from about 48 s. The single-run tests allow 60 s. So under load the timeout can now fail a linear implementation. Known from the decorators and the loop count in _ratio.

@pmazzocchi pmazzocchi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approves head 42cc95d.

  • Relies on a fresh local reviewer's CLEARED f2b5fa5b6. The rebase onto bbb0555 is a range-diff =, so the clearance carries over. Gates rerun on 42cc95d: pre-commit, pytest (100% coverage) and sphinx -W all exit 0.
  • Bot ACK: #2623 (review)
  • Every check passed or was skipped on this head.
  • Not checked: the Sign-off script was not run locally (CI's lint / Sign-off passed). The failure rate at load 20 or more is still unmeasured, as the body says.

@pmazzocchi
pmazzocchi merged commit 547934e into main Oct 11, 2026
22 checks passed
@pmazzocchi
pmazzocchi deleted the tapscript-scaling-2613 branch October 11, 2026 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants