feat: anchor gg_partial_varpro(scale = "prob") at each subject's own level - #301
Merged
Merged
Conversation
partialpro() returns every case at the cohort-mean intercept, so prob's per-case plogis-then-average is not the expected proportion. Shift each non-binary case's curve through its own OOB log-odds at its observed x. On simulated data with heterogeneous case levels this cut the error against the true PD from 0.105 to 0.033 (8/8 seeds). Still to do: roxygen + NEWS, examples that now warn, full suite, and the varpro vignette precompute (pd_iris_multi is on prob). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- roxygen: new "Restoring each subject's level" section; the Jensen example now uses a reproducible simulation (0.13 prob_typical vs 0.35 prob, true 0.38; 0.14 before anchoring) in place of the VA-ECMO figures, which were measured on the unanchored curve. - explainability vignette and the prob_typical NEWS bullet: same numbers, plus a note that "prob" needs object =. - NEWS: user-visible change to "prob" curves. - Examples on mock data (no fit to anchor from) wrap the "prob" call in suppressWarnings() with a comment saying why. - Rebuild varpro precompute: pd_iris_multi moves toward the base rate (Sepal.Width 0.084 -> 0.342 for a balanced 3-class target); other objects change only by the new provenance field or rfsrc ctime. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Ignored arguments can alter anchoring, and unavailable OOB predictions can silently change the averaging cohort.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Anchors classification partial-probability curves to subject-level OOB predictions.
Changes:
- Adds anchoring logic and provenance metadata.
- Adds simulation and behavior tests.
- Updates user-facing documentation and examples.
| File | Description |
|---|---|
R/gg_partial_varpro.R |
Implements anchoring and provenance. |
R/plot.gg_partial_varpro.R |
Updates warning example. |
tests/testthat/test_gg_partial_varpro.R |
Adds anchoring tests. |
vignettes/explainability.qmd |
Explains restored subject levels. |
NEWS.md |
Documents behavior change. |
man/gg_partial_varpro.Rd |
Updates generated extractor documentation. |
man/plot.gg_partial_varpro.Rd |
Updates generated plot documentation. |
Files not reviewed (2)
- man/gg_partial_varpro.Rd: Generated file
- man/plot.gg_partial_varpro.Rd: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #301 +/- ##
==========================================
+ Coverage 90.82% 90.85% +0.02%
==========================================
Files 60 60
Lines 5834 5875 +41
==========================================
+ Hits 5299 5338 +39
- Misses 535 537 +2
🚀 New features to boost your workflow:
|
Copilot review on #301: - With part_dta supplied, '...' is warned as ignored, yet a learner= or newdata= in it still skipped anchoring. Consult '...' only when part_dta is computed in this call. - A case never out of bag has an NA OOB prediction, so its anchor, and then its whole row, went NA and colMeans(na.rm = TRUE) dropped it from the cohort. It now takes its in-bag prediction (14 of 200 cases at ntree = 5 in the new test). Docs: say a precomputed part_dta is anchored whenever object is given, which assumes partialpro's default learner. The vignette precompute is unaffected (its 50-tree iris forest has no never-OOB case). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On R 4.5.3 (CI oldrel-1) partialpro() returned nothing for x1 on the pure-noise outcome, so part_dta = NULL sent the call down the compute path with the dummy learner and it errored. Put signal in x1 and assert partialpro() returned it before relying on it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

What
varPro::partialpro()fits each subject's curve separately, but returns every row at the cohort-mean intercept (yhat.par <- global.mean + tcrossprod(B, Xpow)) and keeps only each subject's slope. On the log-odds scale that puts every subject at the average log-odds, soscale = "prob"(per-subjectplogis, then average) is not the expected proportion it is documented as. Instead it nearly matches"prob_typical".This PR shifts each non-binary subject's curve to pass through its own OOB log-odds at its observed value, then back-transforms and averages. The shape
partialpro()fitted is unchanged. The anchors are clamped at 1e-3, the same aspartialpro's ownmylogodds().scale = "prob"with a classificationobject. This is recorded asprovenance$anchored, which adds one element to the provenance attribute; no class or column names change....passes a customlearnerornewdata, because the forest's OOB predictions wouldn't share a footing with those curves.partialpro()already returns per subject.object, it warns and returns the old curve."prob_typical","odds","logodds"and"surv"are unchanged.Evidence
Simulated
y ~ plogis(1.5·x1 + b·x2), scored against the true partial dependence over 8 seeds:probbeforeprob_typicalprobanchoredVignette precompute, balanced 3-class iris: the flat
Sepal.Widthcurve for virginica moves from 0.084 to 0.342, which is the base rate, as expected for a weak predictor.User-visible change
"prob"has been on CRAN since 3.3.0, so its curves change, most on heterogeneous cohorts. NEWS says so. The Jensen example in the roxygen, the explainability vignette and NEWS used VA-ECMO numbers (0.96 against 0.74) measured on the un-anchored curve. They're replaced with a reproducible simulation: 0.13 underprob_typical, 0.35 underprob, true value 0.38, and 0.14 underprobbefore this change. Anything that quoted the old VA-ECMO numbers needs re-running.Verification
document(),lint_package(): 0 lintsNOT_CRAN=true VDIFFR_RUN_TESTS=true devtools::test(): FAIL 0 | SKIP 6 | PASS 2191. The skips are pre-existing and deliberate, and no snapshot changes.probbeats un-anchored against the true PD; the no-objectwarning; and a custom learner not being anchored.R CMD check --as-cranonmain(b3b3373a, before this PR) came out 0/0/0. This branch touches a vignette, so it should be checked again before the RC.🤖 Generated with Claude Code