Fix multivariate default n_best scaling and mirror R's dependence tryCatch chain - #85
Merged
Merged
Conversation
The default n.best in nns_m_reg used floor((1-dependence)*sqrt(ncol)), mirroring R's Multivariate_Regression.R:129 where n is shadowed by ncol(original.IVs). With sqrt(#features) <= 2.24 for typical widths, the default collapsed to n.best = 1 for any dependence above ~0.1, making order=None predictions identical to order="max" (pure 1-NN) on continuous multivariate data. Use sqrt(nrow) instead, consistent with the sqrt(n_obs) grid nns_stack already cross-validates over. Verified against a live patched R NNS 13.1 build on shared data: order=None now differs from order="max" identically in both languages (59/60 test rows bit-exact, remaining row is the out-of-support gradient-extension path). Full test suite passes; no recorded R-parity case is affected because all cached multivariate fixtures resolve to n.best = 1 under both formulas. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BPbZvtDw4h2XJo9w5hw57h
R Regression.R wraps the NNS.dep call in tryCatch with a 0.1 fallback before averaging with the copula measure; the port only guarded the copula half. Wrap the nns_dep call the same way so the order=NULL dependence chain is a literal transcription of the R block. Verified against live R NNS 13.1 on nine datasets spanning n<100 and n>=100: NNS.dep and NNS.copula components bit-identical, composed dependence within 1 ulp, and every resolved dep.reduced.order equal (order=NULL and integer overrides). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BPbZvtDw4h2XJo9w5hw57h
Merged
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.
Changes
Two fixes to keep
order=Nonebehavior exact against R NNS, verified with a live R 4.3.3 + NNS 13.1 build:1.
_resolve_n_best: scale defaultn_bestbysqrt(n_obs), notsqrt(n_features)(multivariate_regression.py)The default
n_best = max(1, floor((1-dependence)*sqrt(x.shape[1])))mirrored R'sMultivariate_Regression.R:129, wherenis shadowed byncol(original.IVs). Withsqrt(#features) ≤ 2.24, the default collapsed ton_best = 1for any dependence above ~0.1, makingorder=Nonepredictions identical toorder="max"(pure 1-NN) on continuous multivariate data. Now usessqrt(x.shape[0]), consistent with thesqrt(n_obs)gridnns_stackalready cross-validates over. Companion R PR: OVVO-Financial/NNS#39.2.
_regression_dependence: mirror R'stryCatchfallback aroundNNS.dep(regression.py)R wraps the
NNS.depcall intryCatch(..., error = function(e) .1)before averaging with the copula measure; the port only guarded the copula half. Theorder=NULLdependence chain is now a literal transcription of the R block.Verification
n<100halving, weak/strong dependence,dependence==1):NNS.depandNNS.copulacomponents bit-identical (≤6e-16), composed dependence within 1 ulp, all 9/9 resolveddep.reduced.ordervalues equal fororder=Noneand integer overrides.n_best=1under both formulas).nns_regRMSE improves iid 1.39→1.12, support_shift 2.22→2.07, prior_trap 1.58→1.25.🤖 Generated with Claude Code
https://claude.ai/code/session_01BPbZvtDw4h2XJo9w5hw57h
Generated by Claude Code