Make fitted tests backend-neutral (#311, #328) - #349
Merged
Merged
Conversation
Collaborator
Author
|
The pull request body now includes the corrected #328 worker-count description and validation of manually supplied worker counts. The latter rejects fractional, non-finite, out-of-range and non-positive values before integer conversion. |
Collaborator
Author
|
Independent review completed after three cycles. No defect was found in the The review identified and corrected these maintenance problems:
The reviewer accepted the corrections and found no remaining actionable issue |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses #311 and #328.
What
Twelve fitted tests no longer pass
open_progress = FALSEthroughbnec()orbnec_hurdle(). The argument is specific to therstanbackend. Omitting itlets the same tests run when
brmsis configured to usecmdstanr.The branch is based on
devafter #331 merged the #328 fixture reuse. Thatchange builds five Stan programs where the affected files previously built
fourteen. This pull request completes the combined test-infrastructure batch
with #311. It also corrects stale descriptions of #328's withdrawn worker-count
policy and prevents a manually dispatched timing run from truncating a
fractional worker count.
Why it matters
The test suite previously failed when a contributor had
options(brms.backend = "cmdstanr")in the session or profile. The failure wasreported as a model that did not fit, although the model and data were valid.
The retained fixture reuse removes nine repeated compilations without changing
an assertion.
Evidence
On R 4.6.1 with
brms2.23.0,cmdstanr0.9.0 and CmdStan 2.39.0, a revisednec4paramtest fit compiled and sampled through thecmdstanrbackend. Bothchains completed and
bnec()returned abayesnecfit. All eight edited testfiles parse, no
open_progresscall remains undertests/, andR CMD build --no-build-vignettessucceeds.The workflow parses as YAML. Direct tests of its worker-input rule accept
positive integral counts and reject fractional, non-finite, out-of-range and
non-positive values.
The fixture reuse merged in #331 was measured directly: the affected files
compiled five Stan programs rather than fourteen with assertion counts unchanged
at 71 and 36. Its four-platform R CMD check matrix passed. This pull request's
matrix tests the combined state.
Implementation detail
open_progressdefaults tointeractive()for therstansampling path, whichis already false in non-interactive test runs. The
cmdstanrpath does notaccept the argument. Deleting it therefore changes no model equation, family,
prior, seed, chain count, iteration count or assertion.
#328's addendum withdrew the proposed CI worker-count change after paired runs
could not distinguish its effect from runner variance. This branch does not
restore that setting. It retains only #331's deterministic fixture reuse and
the workflow instrumentation that already merged to
dev. A blank dispatchinput leaves
TESTTHAT_CPUSunset, and a supplied value must represent apositive integer before conversion.
No scientific judgement was required for either part.