Align CDNOTS+ with PCMCI+; adopt as run_cdnots_plus defaults - #32
Merged
Conversation
Ported from the internal cdnots-plus-pcmci-align branch. Three changes to CDNOTS+'s contemporaneous-edge handling, all now the library defaults: 1. Phase-2 MCI conditioning (skeleton_discovery.mci_skeleton). CDNOTS+ dropped every lag-copy of a tested variable from the conditioning set; PCMCI+'s _run_pcalg_test drops only the exact tested node. The old rule under-conditions on autocorrelated series, inflating false positives. legacy_mci_conds=True restores it. 2. Collider conflict resolution (uc_sepset priority=1, new default). When two colliders imply opposite orientations, PCMCI+ marks the edge conflicting and drops it (its 'x-x' marker); CDNOTS+ used to tie-break and commit a direction. priority=1 abstains instead, propagated across time-shifted copies, and the resulting bi-directed marker is dropped on conversion rather than emitted as two contradictory directed edges. 3. Nonstationarity orientation (phase_three orient_margin, default 0.1). The sink search took an unconditional argmin and committed every candidate's undirected neighbours -- so 0% of contemporaneous edges were ever left unresolved with the C node present, versus 51% with it absent. The margin gate requires the winning candidate to be separated from the runner-up by at least orient_margin before committing; otherwise the remaining edges stay undirected and are dropped, matching PCMCI+'s treatment of o-o links. Also fixes a related bug in cdnots_to_tigramite_graph: the edge-conversion loop visits both (i,j) and (j,i) and writes in place, so the o-o branch's symmetric output could be read back by the mirrored visit and mistaken for a priority=1 conflict marker under keep_undirected=True. Now decided from a pre-loop snapshot. Measured on ~9.5k paired synthetic runs (three topologies, mean degree 1.5 to 13.5, four dimensions, four sample sizes): CDNOTS+ matches PCMCI+ to within +0.0008 F1 (median difference exactly zero, identical graphs on 53% of instances) while keeping the nonstationarity mechanism PCMCI+ lacks. Against CDNOTS the gain grows with graph density and shrinks with sample size, independent of dimension -- +0.02 to +0.29 F1 depending on topology and density. Full writeup in a forthcoming technical report. Notebook rewritten: two of its three prior findings no longer reproduced (claims that alpha=0.01 was required, and that CDNOTS+ hurt on Erdos-Renyi/small-world) and are replaced with the density-sweep result. The recall-not-precision mechanism finding held up and is kept. New tests cover the previously-unreachable priority=1 path (it used to raise NotImplementedError for any num_lags > 0) and the conversion snapshot fix. All existing tests pass (262 passed; 6 pre-existing failures in test_nan_pairwise.py/test_sigkci.py are unrelated, a compiled sigkernel dependency mismatch present on main before this change). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
…ffected notebooks alpha=0.01 matches PCMCI+'s default and is empirically better for CDNOTS+ in aggregate across the topology sweep (plain CDNOTS keeps its 0.05 default). SigKCIGPU always stored float32 data but the optional sigkernel package's compiled backend requires float64, crashing with a buffer dtype mismatch whenever sigkernel was installed. It also measured slower than the existing pure-torch fallback at the path lengths used here, so the sigkernel fast-path is removed entirely rather than fixing the dtype. Reran every example notebook that calls run_cdnots_plus without pinning the changed defaults (priority, legacy_mci_conds, orient_margin, alpha), verified each rerun against a clean public/main checkout on identical dependency versions to separate real CDNOTS+ drift from unrelated dependency drift, and updated narrative/takeaways text to match the new numbers.
codespell only scans files changed in a PR's diff; this is the first PR to touch sigkci_gpu.py since 'lamda' (intentionally avoiding the 'lambda' keyword) was introduced, so it was never caught before. Renaming it would break the public parameter name, so ignore it instead.
2 tasks
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.
Summary
run_cdnots_plusdefaultscdnots_to_tigramite_graph) that could silently drop undirected edges underkeep_undirected=Trueexamples/cdnots_or_cdnots_plus.ipynb: two of its three prior findings no longer reproduced and are replaced with a density-sweep resultFull detail in the commit message. A technical report with the complete experimental writeup is forthcoming.
Test plan
pytest tests/— 262 passed; 6 pre-existing failures (test_nan_pairwise.py,test_sigkci.py) confirmed present onmainbefore this change (compiledsigkerneldependency mismatch, unrelated)priority=1path and the conversion snapshot fix, all passingblack/isort/flake8clean🤖 Generated with Claude Code