Skip to content

Align CDNOTS+ with PCMCI+; adopt as run_cdnots_plus defaults - #32

Merged
fesanghary merged 3 commits into
mainfrom
feat/cdnots-plus-pcmci-align
Aug 19, 2026
Merged

Align CDNOTS+ with PCMCI+; adopt as run_cdnots_plus defaults#32
fesanghary merged 3 commits into
mainfrom
feat/cdnots-plus-pcmci-align

Conversation

@fesanghary

Copy link
Copy Markdown
Contributor

Summary

  • Three fixes to CDNOTS+'s contemporaneous-edge handling (MCI conditioning, collider conflict abstention, nonstationarity orientation margin), now the run_cdnots_plus defaults
  • Fixes a related edge-conversion bug (cdnots_to_tigramite_graph) that could silently drop undirected edges under keep_undirected=True
  • CDNOTS+ now matches PCMCI+ to within +0.0008 F1 (~9.5k paired synthetic runs) while keeping the nonstationarity mechanism PCMCI+ lacks
  • Rewrites examples/cdnots_or_cdnots_plus.ipynb: two of its three prior findings no longer reproduced and are replaced with a density-sweep result

Full 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 on main before this change (compiled sigkernel dependency mismatch, unrelated)
  • New tests for the previously-unreachable priority=1 path and the conversion snapshot fix, all passing
  • black/isort/flake8 clean
  • Notebook cells execute end to end

🤖 Generated with Claude Code

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>
@fesanghary
fesanghary requested a review from a team as a code owner August 18, 2026 20:07
@github-actions

Copy link
Copy Markdown

⚠️ Deprecation Warning: The deny-licenses option is deprecated for possible removal in the next major release. For more information, see issue 997.

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

…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.
@fesanghary
fesanghary merged commit 519e09c into main Aug 19, 2026
10 checks passed
@fesanghary
fesanghary deleted the feat/cdnots-plus-pcmci-align branch August 22, 2026 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant