Use native NNS distance by default package-wide#127
Merged
OVVO-Financial merged 5 commits intoJul 16, 2026
Conversation
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.
Motivation
distuses the native NNS formula (sum(abs(z) + z**2)) rather than implicitly retaining historical L2 behavior.None/NULLexplicitly so cache keys and R calls distinguish the native default from intentionally-requestedL2.dist="L2"delegations in ordinary package source.Description
dist="L2"to usedist=None(or omit thedistkwarg) in derivative and VAR call paths, primarily insrc/nns/diff.pyandsrc/nns/var.py.tests/_r.pyto acceptdist: str | None = None, include"dist": distin the helper argument dictionaries, decodeargs$disttodist_arg(NULL vs character) inside embedded R scripts, and passdist = dist_argby name intoNNS.reg,NNS.stack, andNNS.boostcalls.tests/parity/so default parity comparisons use Pythondist=Noneand Rdist = NULLinstead of implicit L2; preserved and kept dedicated explicit-L2 tests unchanged where they assert explicit L2 behavior.tests/invariants/test_native_distance_defaults.pythat (1) assert derivative helpers delegate with nativeNone(monkeypatched checks) and (2) scan package source AST to fail if an ordinary package call delegates with a hard-codeddist='L2'.dist="L2"remain as documentation.Testing
ruff check .completed successfully.python -m compileall -q src tests scriptscompleted successfully (AST/compile sanity OK).numpy) and R (Rscript) are not available in this environment, so live R parity and full pytest runs are blocked.python scripts/regenerate_r_cache.py --fresh -- tests/paritycould not run becauseRscriptis not onPATHin this environment and so the R cache could not be regenerated here; the PR includes the helper changes required prior to regeneration but the actual cache rebuild must be performed in the authorized CI/regeneration environment and committed afterwards.mypyandpython -m buildwere attempted but were blocked or produced missing-import diagnostics due to the same unavailable build/dependency tooling; these checks should be re-run in CI where dependencies are installed.Notes on remaining hard-coded
dist="L2"occurrences: the only allowed remaining matches are explicit L2 tests and documentation (for exampletests/invariants/test_native_distance_defaults.pyexplicit-L2 assertions and R manual pages); there are no remaining hard-codeddist="L2"delegations inside ordinary package source after this change, and the new invariant test will detect regressions.If you want, I can (1) run the official R cache regeneration in the proper environment and update
tests/_r_cache.json, or (2) open follow-up CI job output to validate live R parity and cache-only parity once the pipeline runs in CI withnumpyandRinstalled.Codex Task