Dispatch p/q functions once per vector when the parameters are scalar - #202
Draft
joethorley wants to merge 1 commit into
Draft
joethorley wants to merge 1 commit into
joethorley wants to merge 1 commit into
Conversation
`.pdist()` and `.qdist()` called the distribution function once per element of `q` or `p` through `mapply()`, so `ssd_qmulti(1:99 / 100)` rebuilt the mixture, normalised the weights and constructed the CDF closure 99 times, and `pmulti_fun()` resolved each component's function by name and re-ran `purrr::imap()` on every one of the dozen evaluations inside each `uniroot()` solve. When every parameter is a single value, call the distribution function once over the non-missing elements, reproducing the elementwise NA, NaN and invalid-parameter semantics exactly. Parameter vectors, as the bootstrap passes, still dispatch elementwise so that they recycle against the probabilities. Resolve the component functions and separate the weights from the parameters once in `pmulti_fun()`, outside the returned closure. No snapshot changes. `ssd_qmulti(1:99 / 100)` on the `ccme_boron` fit drops from 0.27 s to 0.05 s and a four-proportion, 100-bootstrap `ssd_hc(ci_method = "multi_fixed")` from 9.2 s to 4.7 s. Equivalence tests compare the vectorised and elementwise paths for every distribution over missing, boundary and invalid inputs. Closes #197. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This branch has not been deployed
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
1 participant
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.
Closes #197
Summary
.pdist()and.qdist()dispatched the distribution function once per element ofqorpthroughmapply(). For the model-averaged distribution every one of those scalar calls rebuilt thessd_emulti()skeleton, normalised the weights, constructed the CDF closure and (since #196) derived the support limits, and the closure returned bypmulti_fun()resolved each component'sp*_ssd()by name and re-ranpurrr::imap()on every one of the roughly twelve evaluations inside eachuniroot()solve.Two changes, both behaviour preserving:
.pdist()/.qdist()call the distribution function once over the non-missing elements. NA, NaN and invalid-parameter semantics match the elementwise.pd()/.qd()exactly, and the function is never called for missing elements, as before. Parameter vectors, which the bootstrap passes throughssd_qmulti(), still take the elementwise path so they recycle against the probabilities.pmulti_fun()resolves the component functions and separates weights from parameters once, outside the returned closure, which now does only the weighted sum.Verification
No snapshot changed anywhere in the suite, which is the direct evidence the refactor is behaviour preserving. New tests in
test-pqr.Rcompare the vectorised and elementwise paths for every distribution inssd_dists_all()overNA,NaN,-Inf,0,1,Infand out-of-range inputs, check that invalid scalar parameters giveNaNfor every element, and check that parameter vectors still recycle. Full suite 1427 passing, 0 failures;R CMD check0 errors, 0 warnings, 0 notes; air clean.Timing on the
ccme_boronsix-distribution fit (before / after):ssd_qmulti(1:99 / 100, ...)ssd_hc(ci = TRUE, nboot = 100, ci_method = "multi_fixed"), four proportionsThe remaining bootstrap cost is in refitting, not in quantile dispatch.