Repository navigation
fix: break ties in the k-NN estimators and use Chebyshev when Z is None (follow-up to #14) - #53
Open
Parva-26 wants to merge 1 commit into
Conversation
This branch has not been deployed
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.
Follow-up to #14 and #48. In #14, @jefish003 preferred adding very small noise to break ties. This PR does that and finishes the Chebyshev default for the cases #48 didn't cover.
1. Tie-breaking noise
When a point has k or more exact copies, the distance to its k-th neighbour is 0, the strict
<counts give -1 anddigamma(0)is-inf.knn_mutual_informationthen returnedinfandknn_conditional_mutual_informationreturnednan, for example on Poisson(3) counts with k=3 or on bootstrap resamples. A new helper,_add_tie_breaking_noise, adds independent noise of relative size 1e-10 to X, Y and Z before the neighbour search, as Tigramite's CMIknn and scikit-learn'smutual_info_regressiondo. The seed is fixed, so results stay deterministic. On continuous data the estimates don't change (the numbers in [1] below are identical with and without the noise).2. Chebyshev when Z is None
With
metric=None, theZ=Nonebranch still used Minkowski (p=2) andknn_mutual_informationdefaulted to Euclidean, so the marginal case kept the Euclidean bias: with N=1000 and a true MI of 0.144, the mean estimate was -0.118 for dependent data and -0.264 for independent data, against +0.146 and -0.001 with Chebyshev (script below). Both defaults are now Chebyshev. On a small kNN discovery run (n=4, T=300, 6 seeds) this made little difference to the graph (TP/FP/FN 20/1/5 before, 19/1/6 with Chebyshev), but direct MI calls returned the wrong sign.3. Docstring
The
knn_conditional_mutual_informationdocstring gave the two incorrect identities noted in #14 and described a decomposition the code doesn't use. It now gives the Frenzel-Pompe / Vejmelka-Paluš formula that the code implements, with references.4. Bootstrap intervals with k-NN estimators
bootstrap_cmi_confidence_interval(method="knn")used to raise a ValueError because of NaN replicates (see #40). With the tie fix it runs, but repeated rows in the resamples still bias the estimates: over 20 datasets the 95% interval covered the true CMI 7/20 times forknn, 13/20 forgeometric_knnand 19/20 forgaussian. The function now emits aUserWarningforknnandgeometric_knn. Itsmetricdefault is nowNone, somethod="knn"uses Chebyshev likediscover_network.Tests
New
causationentropy/tests/core/information/test_knn_ties_and_defaults.py: finite estimates with repeated values and count data, determinism, inputs not modified, the Chebyshev default and its accuracy forZ=None, and the bootstrap warning. 6 of the 8 tests fail on current main.Validation
black --check,isort --check-onlyand the flake8 hard gate pass.Not changed: the
discover_networkdocstring still saysmetricdefaults to'euclidean'(out of date since #48), and the internal functions indiscovery.pykeepmetric="euclidean"defaults. I leftdiscovery.pyalone to avoid conflicts with #33 and #46.Script for the numbers above (run from the repo root of this branch; part [2] takes about 4 minutes)