Fix pinned_step for limit-cycle pinning and add driver node search#64
Fix pinned_step for limit-cycle pinning and add driver node search#64xuan-w wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes BooleanNetwork.pinned_step() so pinned variables are written from pinned_binstate (enabling correct limit-cycle pinning in pcstg construction), and adds a new minimum driver-set search API for pinning control.
Changes:
- Fix
pinned_step()semantics to write pinned outputs frompinned_binstate(while still computing unpinned updates frominitial). - Add
_signatures_distinguish_attractors()pre-filter andpinning_control_driver_nodes()brute-force search starting atceil(log2(N_attractors)). - Add a comprehensive new test module covering pinned-step semantics, pcstg connectivity invariants, pcf/mpcf correctness, and driver-node search.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
cana/boolean_network.py |
Implements signature pre-filter + minimum driver-set search; fixes pinned_step to honor pinned_binstate; renames pcstg loop variables for clarity. |
tests/test_pinning_control.py |
Adds new regression and behavior tests for pinned stepping, pcstg construction, pcf/mpcf, and driver-node search. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…t pcstg sufficiency check This PR completes Alex Gates' 2018 pinning-controllability design, fixes a long-standing bug in pinned_step, and restores two utility functions for minimum-driver-set search with a strict pcstg sufficiency criterion. The pinned_step bug fix ----------------------- The pcstg edge-loop scaffolding for limit-cycle attractors has been in place since Alex Gates' 2018 introduction of pinning controllability (98792ed): the iteration over attractor STG edges, the (attsource, attsink) pair, and the call-site form pinned_step(initial, pinned_binstate=attsink, pinned_var=...). What was missing was the body of pinned_step that would honor the pinned_binstate argument — pinned positions in the output were copied from initial instead of being written to pinned_binstate. For fixed-point attractors src and sink share the same pin pattern, so the missing-body path happened to produce correct results. For limit-cycle attractors where the pinned variable flips between cycle states, src and sink differ; the buggy body kept pin=src in the output, fragmenting the pcstg into one disjoint snapshot per cycle position. The cycle's states were split across snapshots, set(att) was never a subset of any single weakly-connected component, and fraction_pinned_configurations returned 0 for every such attractor. A related independent fix was made in 2019 (44a96d3, on python3-friendly) via a different mechanism — splitting pinned_step into two functions and changing the call site. That fix shipped on its branch but didn't make it to upstream when subsequent merges branched off pre-fix history. This commit completes Alex's original design: the body of pinned_step now writes pinned_binstate into the destination's pinned positions while still using the source pin pattern (from initial) to compute the unpinned step. The call site stays exactly as it was written in 2018. Changes ------- * pinned_step: body now reads pin from `initial` (as inputs to node update functions) and writes pin from `pinned_binstate`. Signature unchanged from Alex's original. Docstring rewritten to document all three arguments correctly (the original docstring had a phantom `n` arg from a copy-paste and never mentioned `pinned_binstate`). * pinning_controlled_state_transition_graph: rename loop variables attsource/attsink → src_pin/dst_pin. Same variables, just renamed — the old names suggested *attractor states* when they actually hold the *pin-bit projections* of those states. The rename is purely internal: these are local names inside one method's body, not part of any signature, attribute, or external API. An inline comment in the function explains the meaning and notes the rename. Restored search and Stage-2 design ---------------------------------- pinning_control_driver_nodes — minimum-driver-set search via exhaustive enumeration bracketed by ceil(log2(N_attractors)) lower bound and FVS upper bound. Discrete/Boolean analog of FVS-based open-loop control (Mochizuki & Fiedler 2013, JTB §7). The search uses a two-stage filter: * Stage 1 (necessary) — _signatures_distinguish_attractors. Cheap pre-check that the candidate's per-attractor signatures are distinct. Handles fixed-point, pin-constant cycle, and pin-flipping cycle attractors separately. * Stage 2 (strict sufficient) — attracting_components(pcstg) == [set(att)]. Each pcstg must have exactly one attracting SCC, equal to the target attractor's state set. Pure integer/set comparison — no floating-point exposure even at large pcstg sizes. The historically-tempting weaker criterion is WCC == 1 per pcstg. WCC=1 is necessary (no orphan basins) but not sufficient: it only verifies the pcstg is connected, not that the pcstg's attractor equals the intended target. For deterministic pcstg (fixed-point pinning) each WCC has exactly one attracting SCC, but that SCC may be a different attractor than the target if the pin pattern's projection collides with multiple original attractors and the unpinned dynamics converge to one of the others. Concrete example: Arabidopsis thaliana under pin = {AP3, UFO, AP1, LFY, WUS} — fpc = [1, 0, 1, 1, 1, 0, 1, 1, 1, 0] across the ten attractors, so three target attractors are not the pcstg's attractor under that pinning. Yet all ten pcstg are single-WCC. The size-6 sets Alex Gates' 2018 slide-3 reports for Thaliana pass the strict check; the WCC=1-only check would have admitted the size-5 set as a false positive. Drosophila and Yeast cell-cycle happen to be robust to the difference because their pin signatures distinguish all attractors uniquely. _signatures_distinguish_attractors — module-level helper used by the Stage-1 filter above. Three attractor types handled inline. A third orphaned function from a 2019 effort (full_control_driver_nodes) is deferred to a separate PR. The right name (configuration_perturbation_driver_nodes) needs lab consensus on the broader 4-quadrant control taxonomy (perturbation/pinning × attractor-level/configuration-level). Tests ----- 20 new tests in tests/test_pinning_control.py: - 18 tests on a 3-node BN fixture (A *= B; B *= A; C *= A and B) with all three attractor types simultaneously (length-2 cycle, two fixed points). Exercises: * bug surface (pin A or {A,B} — pin flips in cycle): pcstg single-WCC + mpcf=1.0; verifies the bug is fixed without tripping over fixed-point cases that incidentally worked pre-fix. * pin-constant control case (pin C — pin invariant in cycle): values unchanged from pre-fix; guards against regressing the fixed-point-equivalent case. * minimum driver-set search: returns {A,B} as the unique 2-element pinning control set (ceil(log2(3))=2 lower bound). * pinned_step unit tests: pin-unchanged (pinned_binstate matches initial[pinned_var]), pin-advanced (pinned_binstate differs), multi-pin, length-mismatch error. * _signatures_distinguish_attractors: four cases covering distinct fixed points, colliding fixed points, pin-constant cycle, and pin-flipping cycle. - 2 real-network regression tests on Arabidopsis thaliana (Chaos 2006), the network that motivated the strict Stage-2 criterion: * test_pinning_control_driver_nodes_thaliana_strict_criterion: asserts the search returns exactly Alex's three size-6 sets. * test_thaliana_size5_false_positive_rejected_by_strict_check: direct assertion on the strict criterion against the historical false-positive size-5 set; lighter-weight regression that survives even if the search algorithm changes shape. All 20 new tests pass; full CANA test suite (81 tests total) passes. Sanity check on real CC model: Fanconi anemia (length-2 cycle with CHKREC flipping), pinning CHKREC → pcf=1.0 (pcstg single WCC, correct full-control on the cycle). Lac Operon (fixed-point only) — unchanged. References ---------- - Fiedler, Mochizuki, Kurosawa, Saito (2013). Dynamics and Control at Feedback Vertex Sets I. J. Dyn. Diff. Equat. 25, 563-604. - Mochizuki, Fiedler, Kurosawa, Saito (2013). Dynamics and Control at Feedback Vertex Sets II. J. Theor. Biol. 335, 130-146. - Zañudo, Yang, Albert (2017). Structure-based control of complex networks with nonlinear dynamics. PNAS 114, 7234-7239. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
xuan-w
force-pushed
the
restore-pinning-control-driver-nodes
branch
from
8d3da1d to
b788ffc
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Return [()] for single-attractor networks and tuple in fallback for consistent list-of-tuples return type - Exclude constant nodes from driver-set search (matching attractor_driver_nodes); add comment about modifying models if a constant node needs to be controllable - Bound search loop by len(nodeids) instead of Nnodes to avoid no-op iterations when constants are excluded - Clarify _signatures_distinguish_attractors docstring: collisions among flipping cycles are not checked (caught downstream) - Replace bare asserts in pinned_step with ValueError for clearer error messages - Look up attractors by content in tests instead of hard-coded indices to avoid dependence on implementation-defined ordering Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
xuan-w
force-pushed
the
restore-pinning-control-driver-nodes
branch
from
4a42aa9 to
3929506
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Build a quick lookup so the comprehension is O(Nnodes) | ||
| # rather than O(Nnodes * |pinned_var|). | ||
| pin_map = dict(zip(pinned_var, pinned_binstate)) | ||
| return "".join( | ||
| [ | ||
| str(node.step("".join(initial[j] for j in self.logic[i]["in"]))) | ||
| if not (i in pinned_var) | ||
| else initial[i] | ||
| for i, node in enumerate(self.nodes, start=0) | ||
| ] | ||
| pin_map[i] | ||
| if i in pin_map | ||
| else str(node.step("".join(initial[j] for j in self.logic[i]["in"]))) | ||
| for i, node in enumerate(self.nodes, start=0) |
There was a problem hiding this comment.
pinned_step builds pin_map from zip(pinned_var, pinned_binstate). If pinned_var contains duplicates, the dict will silently drop earlier entries and ignore some characters of pinned_binstate, producing incorrect output without an error. Consider validating pinned_var has unique indices (e.g., len(set(pinned_var)) == len(pinned_var)) and raising ValueError when duplicates are present.
There was a problem hiding this comment.
pinned_var is only produced internally by itertools.combinations (guaranteed unique) or by pinning_controlled_state_transition_graph which also assumes unique indices. Adding validation for an impossible case would be defensive noise.
| def test_pinning_control_driver_nodes_thaliana_strict_criterion(): | ||
| """Search must apply the strict criterion (unique attracting SCC | ||
| equals target), not the weaker WCC==1. Returns Alex Gates' three | ||
| size-6 sets, not the historical size-5 false positive.""" | ||
| from cana.datasets.bio import THALIANA | ||
|
|
||
| bn = THALIANA() | ||
| result = bn.pinning_control_driver_nodes() | ||
| names = [n.name for n in bn.nodes] | ||
| sets = {frozenset(names[i] for i in dvs) for dvs in result} | ||
|
|
||
| # Alex Gates 2018 slide-3 result (three alternative size-6 sets, | ||
| # all sharing {UFO, WUS, AP3, AG} and extended by one of three pairs): | ||
| expected = { | ||
| frozenset({"UFO", "WUS", "AP3", "AG", "AP1", "LFY"}), | ||
| frozenset({"UFO", "WUS", "AP3", "AG", "TFL1", "EMF1"}), | ||
| frozenset({"UFO", "WUS", "AP3", "AG", "TFL1", "LFY"}), | ||
| } | ||
| assert sets == expected | ||
| assert all(len(dvs) == 6 for dvs in result) |
There was a problem hiding this comment.
This test module adds a full pinning_control_driver_nodes() search on the THALIANA dataset (noted as ~15s). With no existing pytest slow/benchmark markers in the repo, this will run on every CI invocation and can significantly increase suite runtime. Consider guarding it (e.g., pytest.skip unless an env var is set) or restructuring it to a cheaper regression (similar to the size-5-set strict-check test below), so the default unit test suite stays fast.
There was a problem hiding this comment.
The Thaliana test runs in ~15s and is the sole real-network regression test in this module — it's what caught the WCC-vs-attracting-SCC bug. Adding a slow marker or skip guard for 15s of CI time isn't worth the risk of the regression going untested.
|
@xuan-w, what is the priority in merging this to the master branch intead of its own dedicated branch? |
Hi @fxcosta-phd Sorry for the late reply. That is a good question. I think the history is this: we published the result in PNAS Gates et.al 2021 paper, in SI sec 6. The function to calculate the "real" (in contrast to FVC or other prediction methods) pinning control driver nodes set was never fully merged to CANA. Alex wrote one version, I wrote one version, and neither got fully merged to CANA when we merged everyone's work. I lean toward to merge to the main branch, here are my justifications:
If you think we should hold on, I can also send this to another branch. |
Summary
pinned_step()where pinned variables were copied frominitialinstead of written frompinned_binstate, breaking limit-cycle pinning control (pcstg fragmented into disconnected snapshots,fraction_pinned_configurationsreturned 0)pinning_control_driver_nodes()for minimum-size driver set search withceil(log2(N_attractors))lower bound and a cheap necessary-condition pre-filter (_signatures_distinguish_attractors)attsource/attsink→src_pin/dst_pinin pcstg construction for claritypinned_stepsemantics, pcstg WCC invariants, fraction_pinned_configurations, driver node search, and the pre-filterTest plan
test_pinning_control.pyall pass🤖 Generated with Claude Code