Skip to content

feat: anchor gg_partial_varpro(scale = "prob") at each subject's own level - #301

Merged
ehrlinger merged 4 commits into
mainfrom
feat/prob-anchored
Sep 29, 2026
Merged

ehrlinger merged 4 commits into
mainfrom
feat/prob-anchored

Conversation

@ehrlinger

Copy link
Copy Markdown
Owner

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, so scale = "prob" (per-subject plogis, 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 as partialpro's own mylogodds().

  • Runs only for scale = "prob" with a classification object. This is recorded as provenance$anchored, which adds one element to the provenance attribute; no class or column names change.
  • Skipped when ... passes a custom learner or newdata, because the forest's OOB predictions wouldn't share a footing with those curves.
  • Skipped for binary variables, which partialpro() already returns per subject.
  • Without 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:

spread (b) prob before prob_typical prob anchored anchored better
0.5 0.060 0.063 0.045 7/8
3 0.105 0.130 0.033 8/8

Vignette precompute, balanced 3-class iris: the flat Sepal.Width curve 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 under prob_typical, 0.35 under prob, true value 0.38, and 0.14 under prob before this change. Anything that quoted the old VA-ECMO numbers needs re-running.

Verification

  • document(), lint_package(): 0 lints
  • NOT_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.
  • New tests cover: curves pass through their anchors with the shape kept and binary variables untouched; anchored prob beats un-anchored against the true PD; the no-object warning; and a custom learner not being anchored.
  • R CMD check --as-cran on main (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

ehrlinger and others added 2 commits September 29, 2026 13:16
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>
Copilot AI balanced review requested due to automatic review settings September 29, 2026 17:23

Copilot AI 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.

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 Medium severity

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.

Comment thread R/gg_partial_varpro.R Outdated
@codecov

codecov Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.12195% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.85%. Comparing base (b3b3373) to head (507778b).

Files with missing lines Patch % Lines
R/gg_partial_varpro.R 95.12% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            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     
Files with missing lines Coverage Δ
R/plot.gg_partial_varpro.R 92.60% <ø> (ø)
R/gg_partial_varpro.R 96.59% <95.12%> (-0.22%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ehrlinger and others added 2 commits September 29, 2026 14:00
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>
@ehrlinger
ehrlinger merged commit c042902 into main Sep 29, 2026
10 checks passed
@ehrlinger
ehrlinger deleted the feat/prob-anchored branch September 29, 2026 18:31
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