Fix matchindex() tie breaking#758
Open
stefvanbuuren wants to merge 2 commits into
Open
Conversation
Previously, matchindex() shuffled the donor pool once per call, so all target cases sharing the same (or nearly the same) predicted value drew from the same fixed subset of donors within one imputed dataset. This inflated between-imputation variance and produced overly conservative confidence intervals under Rubin's rules, most visibly in intercept-only PMM models and models with few categorical predictors. Ties among donors are now broken independently for each target case: after the existing one-time sort, contiguous tie blocks are identified, and when a target's k-nearest-neighbour window falls inside a block, either a fast uniform draw (block >= k) or per-step reservoir sampling (block < k) is used instead of the old fixed shuffle order. See #757, following up on the discussion in #236.
- Update matchindex() roxygen (and regenerated RcppExports.R / matchindex.Rd) to describe the tie-block detection, the fast path for large tie blocks, and the historical bug fixed in #757. - Note in mice.impute.pmm()'s use.matcher docs that the legacy matcher() is itself miscalibrated on tied donors, so it should only be used to reproduce results from older mice versions, not as a "safe" fallback.
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.
Fixes miscalibrated donor selection in
matchindex(), the default PMM matching engine since mice 3.11.8, when many target cases share the same (or nearly the same) predicted value — e.g. intercept-only models, few categorical predictors, or heavily rounded outcomes.Root cause: ties among donors were broken once per call (once per imputed dataset), so all tied target cases drew from the same fixed donor subset within one completed dataset, inflating between-imputation variance and producing overly conservative CIs under Rubin's rules.
Fix: after the existing sort, contiguous tie blocks are identified once; each target's tie-breaking is now resolved independently, either via a fast uniform draw (when the near block already has ≥ k donors) or per-step reservoir sampling within blocks smaller than k.
Closes #757. See also the original discussion in #236.
Testing: added
tests/testthat/test-matchindex.R(14 tests, verified to fail 2/14 against the pre-fix code); full existing PMM suite passes; calibration checked via simulation (coverage ~98% vs. ~100% pre-fix, ~0% for the legacymatcher()on the same tied-donor scenario).Opening on a feature branch to gather feedback/testing from users before merging, given the statistical sensitivity of this change.