diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..b9e01cc --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,47 @@ +# Changelog + +This project follows a lightweight "keep a log" style. + + +## 0.1.1 - Refactoring + +- **Deterministic iteration** + - All public iteration APIs (`nodes`, `edges`, `neighbors`, `successors`, `predecessors`) now yield results in a deterministic order. + - Ordering is lexicographic when objects are mutually comparable; otherwise a stable fallback order is used. + +- **Read-only attribute views** + - `get_node_data()` and `get_edge_data()` now return **read-only live views** (mapping proxies) instead of mutable dicts. + - Use `set_node_attrs()` / `set_edge_attrs()` to update attributes (these bump `data_version`). + +- **Edge keys** + - Auto-generated edge keys are now deterministic and local to a `(u, v)` pair (mirrors NetworkX `new_edge_key`). + - Explicit keys must be Python integers (bool is rejected). + +- **New APIs** + - `edges(..., tvec=True)` can include the structural translation vector in iteration records. + - `in_neighbors(...)` and `in_neighbors_inst(...)` provide deterministic access to incoming periodic edges. + - `PeriodicGraph.check_invariants()` validates undirected pairing invariants. + +- **Lattice/SNF** + - Removed the SymPy dependency by implementing exact inversion of unimodular matrices. + +- **Version semantics** + - Pure data-only `data_version` semantics: `data_version` increments only on user-attribute updates that do not change structure (e.g., `set_node_attrs`, `set_edge_attrs`, or `add_node` on an existing node). + - Creating new nodes/edges with attributes does not increment `data_version` (structural change only). + +- **Docs clarifications** + - Clarified that component extraction and weak-neighbor helpers rely on deterministic (stable-sorted) iteration, not insertion order. + - Documented an edge-iteration gotcha for self-loop periodic edges: use `tvec=True` with `keys=True` to disambiguate paired realizations. + - Corrected `SNFDecomposition.diag` documentation (returned length is `rank`). + +- **Performance** + - Reduced redundant generator collection for undirected components by deduplicating paired directed realizations (no semantic change). + +## 0.1.0 — Initial release + +- First public release of **pbcgraph**, a lightweight Python library for periodic graphs built on top of **NetworkX**. +- Provides periodic graph containers with **integer translation vectors** on directed edges to represent connectivity between periodic images. +- Supports **directed/undirected** and **simple/multi** variants (NetworkX `DiGraph`/`MultiDiGraph`-style API). +- Includes core algorithms for **connected components**, **quotient shortest paths**, and basic periodic graph traversal utilities. +- Implements **periodic component** analysis (computing translation subgroup invariants via **Smith normal form**-based lattice reduction). +- Ships with initial tests and basic documentation. diff --git a/README.md b/README.md index 4b86ee8..b4b1b20 100644 --- a/README.md +++ b/README.md @@ -55,6 +55,11 @@ G = PeriodicGraph(dim=2) # Undirected edges are stored internally as two directed realizations # with tvec and -tvec. + +# Self-loop periodic edges are supported (quotient bond to a periodic image): +G1 = PeriodicGraph(dim=1) +G1.add_edge('A', 'A', tvec=(1,)) + G.add_edge('A', 'B', tvec=(0, 0)) G.add_edge('B', 'C', tvec=(0, 0)) G.add_edge('C', 'A', tvec=(1, 0)) # closes a periodic cycle (rank-1 along x) diff --git a/docs/changelog.md b/docs/changelog.md new file mode 100644 index 0000000..5935154 --- /dev/null +++ b/docs/changelog.md @@ -0,0 +1 @@ +---8<-- "CHANGELOG.md" diff --git a/docs/general/concepts.md b/docs/general/concepts.md index adca304..c563bee 100644 --- a/docs/general/concepts.md +++ b/docs/general/concepts.md @@ -32,7 +32,10 @@ This gives you a precise infinite graph semantics without materializing the infi - `u -> v` with translation `tvec` - `v -> u` with translation `-tvec` -Both realizations share a single live attribute mapping. +Both realizations share the same underlying user-attributes dict. + +Note that `get_edge_data()` returns a **read-only live view** of that mapping +(so updating attributes must be done via `set_edge_attrs()`). ### Important invariant (PeriodicGraph) @@ -47,12 +50,22 @@ pbcgraph distinguishes two container families: Here, the translation vector is treated as part of the edge identity. - `PeriodicMultiDiGraph` / `PeriodicMultiGraph`: allow multiple edges per `(u, v, tvec)`. -In all cases, edges are addressed by `(u, v, key)`. Edge keys are intended to be integers. -If you explicitly provide a non-integer hashable key, it will be forwarded to the underlying NetworkX graph, -but pbcgraph only guarantees deterministic key generation and ordering for integer keys. +In all cases, edges are addressed by `(u, v, key)`. +Edge keys must be Python integers (bool is rejected). - If you do not provide a key, pbcgraph generates a deterministic fresh integer key. -- A `key` is shared between the two realizations of an undirected edge in `PeriodicGraph` / `PeriodicMultiGraph`. +- A *base key* is shared between the two realizations of an undirected edge in `PeriodicGraph` / `PeriodicMultiGraph`. + + + +#### Internal representation for self-loops + +A crystallographically common pattern is a *quotient self-loop* with non-zero translation (\`u == v\`, \`tvec != 0\`), representing a bond to a periodic image. + +NetworkX identifies edges by \`(u, v, key)\`, so the two directed realizations of such an undirected edge would collide if they shared the same key. + +To support this, pbcgraph stores undirected realizations using private internal keys derived from the base key (a pair \`_UKey(base, +1)\` / \`_UKey(base, -1)\`). The public API always exposes only the base key (an int). + ## Attributes and version counters @@ -61,8 +74,8 @@ pbcgraph tracks two counters: - `structural_version`: increments on node/edge add/remove. - `data_version`: increments on attribute updates *performed via pbcgraph APIs*. -`get_node_data()` / `get_edge_data()` return the underlying mutable mapping. -Direct mutation of that mapping is allowed but may not update `data_version` (treat as undefined behavior). +`get_node_data()` / `get_edge_data()` return a read-only live view of the underlying mapping. +Direct mutation is not allowed; use `set_node_attrs()` / `set_edge_attrs()`. ## Components, rank, and torsion diff --git a/docs/general/graph_types.md b/docs/general/graph_types.md index d14300b..0764d54 100644 --- a/docs/general/graph_types.md +++ b/docs/general/graph_types.md @@ -34,9 +34,12 @@ Typical examples: Notes: - `PeriodicGraph` is implemented as two directed realizations (`u -> v` with `tvec` and `v -> u` with `-tvec`) - that share one live attributes mapping. + that share the same underlying user-attributes dict. The public API returns + read-only live views of that mapping (update via `set_edge_attrs()`). - Edge identity includes the translation vector: two edges between the same pair of nodes are allowed if their `tvec` differ. +- Self-loop periodic edges are supported: a quotient edge with `u == v` and `tvec != 0` represents a bond to a periodic image. Internally this uses private keys derived from the base key, but the public API still exposes only integer base keys. +- When iterating edges with `keys=True`, pass `tvec=True` for self-loop periodic edges to distinguish the paired realizations; otherwise `edges(keys=True)` can yield duplicate `(u, u, key)` records. ## `PeriodicMultiGraph` diff --git a/mkdocs.yml b/mkdocs.yml index d7a3ba4..5231e6f 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -24,6 +24,8 @@ markdown_extensions: # Math rendering (MathJax) - pymdownx.arithmatex: generic: true + - pymdownx.snippets: + check_paths: true extra_javascript: - javascripts/mathjax.js @@ -65,3 +67,4 @@ nav: - Algorithms: api/algorithms.md - Types: api/types.md - Exceptions: api/exceptions.md + - Changelog: changelog.md diff --git a/pyproject.toml b/pyproject.toml index 89077f3..27dfb22 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -35,7 +35,6 @@ classifiers = [ dependencies = [ "networkx>=3.2", "numpy>=1.23", - "sympy>=1.12", ] [project.optional-dependencies] diff --git a/src/pbcgraph/__about__.py b/src/pbcgraph/__about__.py index 9902dcb..27573f3 100644 --- a/src/pbcgraph/__about__.py +++ b/src/pbcgraph/__about__.py @@ -1,4 +1,4 @@ -__version__ = '0.1.0' +__version__ = '0.1.1' __all__ = [ '__version__', diff --git a/src/pbcgraph/__init__.py b/src/pbcgraph/__init__.py index 250d0a1..984043d 100644 --- a/src/pbcgraph/__init__.py +++ b/src/pbcgraph/__init__.py @@ -39,6 +39,15 @@ G.add_node('A') G.add_edge('A', 'A', tvec=(1,)) # A(i) -> A(i+1) # For an undirected representation, use PeriodicGraph + + # A crystallographically common pattern is a quotient self-loop with + # non-zero translation (bond to a periodic image): + + from pbcgraph import PeriodicGraph + + G = PeriodicGraph(dim=1) + G.add_edge('A', 'A', (1,), kind='bond') + # (adds both directions). Algorithms diff --git a/src/pbcgraph/alg/_neighbors.py b/src/pbcgraph/alg/_neighbors.py index cd32fd0..aa0391c 100644 --- a/src/pbcgraph/alg/_neighbors.py +++ b/src/pbcgraph/alg/_neighbors.py @@ -33,8 +33,9 @@ def weak_neighbors(G: _SupportsPredSucc, u: NodeId) -> List[NodeId]: The weak neighborhood treats the directed quotient graph as undirected. Order is deterministic: - 1) successors in insertion order, - 2) then predecessors in insertion order (excluding nodes already yielded). + 1) successors in deterministic order as provided by `G.successors(u)`, + 2) then predecessors in deterministic order as provided by + `G.predecessors(u)` (excluding nodes already yielded). Args: G: A graph providing `successors(u)` and `predecessors(u)`. diff --git a/src/pbcgraph/alg/components.py b/src/pbcgraph/alg/components.py index a6beb8d..680652d 100644 --- a/src/pbcgraph/alg/components.py +++ b/src/pbcgraph/alg/components.py @@ -25,8 +25,11 @@ def components(G: PeriodicDiGraphLike) -> List[PeriodicComponent]: G: A periodic graph container (structural protocol). Returns: - List of PeriodicComponent objects, ordered by first-seen node in - insertion order. + List of PeriodicComponent objects in deterministic order. + + The order follows the graph's deterministic node iteration + (`G.nodes(...)`), so the first component is the one containing the + smallest node under the container's stable ordering. """ visited: Set[NodeId] = set() out: List[PeriodicComponent] = [] diff --git a/src/pbcgraph/component.py b/src/pbcgraph/component.py index a6f6d6c..ca95478 100644 --- a/src/pbcgraph/component.py +++ b/src/pbcgraph/component.py @@ -23,6 +23,7 @@ ) from pbcgraph.core.exceptions import StaleComponentError +from pbcgraph.core.ordering import fallback_key from pbcgraph.core.types import ( NodeId, NodeInst, @@ -266,9 +267,6 @@ def _compute_potentials(self) -> Dict[NodeId, TVec]: pot: Dict[NodeId, TVec] = {self.root: zero_tvec(dim)} q = deque([self.root]) - # Access internal MultiDiGraph for incoming-edge data. - gnx = getattr(self.graph, '_g') - while q: u = q.popleft() pu = pot[u] @@ -283,20 +281,15 @@ def _compute_potentials(self) -> Dict[NodeId, TVec]: q.append(v) # Incoming edges next (weak traversal). - pred_adj = gnx.pred[u] - for v in pred_adj: + for v, t_in, _k in self.graph.in_neighbors( + u, keys=True, data=False + ): if v not in self.nodes: continue - kd = pred_adj[v] - for k in kd: - if v in pot: - break - ed = kd[k] - t_in = tuple(ed['_tvec']) - pot[v] = add_tvec(pu, neg_tvec(t_in)) - q.append(v) - # If v was added via some incoming edge, - # stop scanning keys for v. + if v in pot: + continue + pot[v] = sub_tvec(pu, t_in) + q.append(v) if len(pot) != len(self.nodes): # This should never happen if component extraction is correct. missing = [u for u in self.nodes if u not in pot] @@ -308,11 +301,49 @@ def _compute_potentials(self) -> Dict[NodeId, TVec]: def _compute_generators(self, pot: Dict[NodeId, TVec]) -> List[TVec]: gens: List[TVec] = [] - for u, v, k in self.graph.edges(keys=True, data=False): + + if not self.graph.is_undirected: + for u, v, t, _k in self.graph.edges( + keys=True, data=False, tvec=True + ): + if u not in self.nodes or v not in self.nodes: + continue + g = sub_tvec(add_tvec(pot[u], t), pot[v]) + if _tvec_is_zero(g): + continue + gens.append(g) + return gens + + def _node_leq(a: NodeId, b: NodeId) -> bool: + try: + return a <= b # type: ignore[operator] + except TypeError: + return fallback_key(a) <= fallback_key(b) + + seen = set() + for u, v, t, k in self.graph.edges(keys=True, data=False, tvec=True): if u not in self.nodes or v not in self.nodes: continue - t = self.graph.edge_tvec(u, v, k) - g = sub_tvec(add_tvec(pot[u], t), pot[v]) + + tv = tuple(int(x) for x in t) + if u == v: + tv_abs = min(tv, neg_tvec(tv)) + ident = (u, u, tv_abs, int(k)) + if ident in seen: + continue + seen.add(ident) + g = tv_abs + else: + if _node_leq(u, v): + a, b, tv_use = u, v, tv + else: + a, b, tv_use = v, u, neg_tvec(tv) + ident = (a, b, tv_use, int(k)) + if ident in seen: + continue + seen.add(ident) + g = sub_tvec(add_tvec(pot[a], tv_use), pot[b]) + if _tvec_is_zero(g): continue gens.append(g) diff --git a/src/pbcgraph/core/ordering.py b/src/pbcgraph/core/ordering.py new file mode 100644 index 0000000..17c2398 --- /dev/null +++ b/src/pbcgraph/core/ordering.py @@ -0,0 +1,85 @@ +"""Deterministic ordering helpers. + +NetworkX iteration order is insertion-based and can vary with construction +sequence. pbcgraph enforces deterministic ordering at the container level. + +Strategy +-------- +For a collection of objects we first try native sorting (``sorted(items)``). +If objects are not mutually comparable (``TypeError``), we fall back to a +stable composite key based on type identity and ``repr``. + +The fallback is deterministic provided that: + +- the type's module and qualname are stable (normally true), and +- ``repr(obj)`` is stable across processes and does not embed memory + addresses. + +For best cross-process determinism, prefer primitive node ids (``int``, +``str``, tuples of primitives). +""" + +from __future__ import annotations + +from typing import ( + Any, Iterable, List, Sequence, Tuple, TypeVar, +) + + +T = TypeVar('T') + + +def fallback_key(x: Any) -> Tuple[str, str, str]: + """Return a stable fallback key for objects that are not comparable.""" + tp = type(x) + return (tp.__module__, tp.__qualname__, repr(x)) + + +def stable_sorted(items: Iterable[T]) -> List[T]: + """Return a deterministically sorted list. + + This tries native ordering first and falls back to a composite key. + """ + seq = list(items) + try: + return sorted(seq) + except TypeError: + return sorted(seq, key=fallback_key) # type: ignore[arg-type] + + +def stable_unique_sorted(items: Iterable[T]) -> List[T]: + """Return unique items in deterministic order.""" + return stable_sorted(set(items)) + + +def stable_tvec(tvec: Sequence[Any]) -> Tuple[int, ...]: + """Canonicalize a translation vector to a tuple of Python ints.""" + return tuple(int(x) for x in tvec) + + +def try_sort_edges( + records: List[Tuple[Any, Any, Tuple[int, ...], int, Any]], +) -> None: + """In-place deterministic sort for edge records. + + Records are ``(u, v, tvec, key, payload)``. + """ + try: + records.sort(key=lambda r: (r[0], r[1], r[2], r[3])) + except TypeError: + records.sort( + key=lambda r: (fallback_key(r[0]), fallback_key(r[1]), r[2], r[3]) + ) + + +def try_sort_neighbor_edges( + records: List[Tuple[Any, Tuple[int, ...], int, Any]], +) -> None: + """In-place deterministic sort for neighbor-edge records. + + Records are ``(v, tvec, key, payload)``. + """ + try: + records.sort(key=lambda r: (r[0], r[1], r[2])) + except TypeError: + records.sort(key=lambda r: (fallback_key(r[0]), r[1], r[2])) diff --git a/src/pbcgraph/core/protocols.py b/src/pbcgraph/core/protocols.py index 41ac6d6..99ebac3 100644 --- a/src/pbcgraph/core/protocols.py +++ b/src/pbcgraph/core/protocols.py @@ -1,20 +1,29 @@ -"""Structural protocols for algorithm modules.""" +"""Structural protocols for algorithm modules. + +Algorithms in :mod:`pbcgraph.alg` and +:class:`~pbcgraph.component.PeriodicComponent` +operate on a narrow "graph-like" surface. This keeps algorithms independent +from the underlying NetworkX backend. + +These protocols are intentionally minimal and may grow between versions. +""" from __future__ import annotations from typing import Iterable, Protocol -from pbcgraph.core.types import NodeId +from pbcgraph.core.types import EdgeKey, NodeId, TVec class PeriodicDiGraphLike(Protocol): - """Protocol for pbcgraph containers used by algorithms.""" + """Protocol for periodic graph containers used by algorithms.""" dim: int structural_version: int data_version: int is_undirected: bool + # Nodes def has_node(self, u: NodeId) -> bool: ... @@ -26,3 +35,22 @@ def successors(self, u: NodeId) -> Iterable[NodeId]: def predecessors(self, u: NodeId) -> Iterable[NodeId]: ... + + # Directed edge access + def neighbors( + self, u: NodeId, keys: bool = False, data: bool = False + ) -> Iterable: + ... + + def in_neighbors( + self, u: NodeId, keys: bool = False, data: bool = False + ) -> Iterable: + ... + + def edges( + self, keys: bool = False, data: bool = False, tvec: bool = False + ) -> Iterable: + ... + + def edge_tvec(self, u: NodeId, v: NodeId, key: EdgeKey) -> TVec: + ... diff --git a/src/pbcgraph/core/types.py b/src/pbcgraph/core/types.py index 7faaf3d..dbd4786 100644 --- a/src/pbcgraph/core/types.py +++ b/src/pbcgraph/core/types.py @@ -11,7 +11,11 @@ from __future__ import annotations -from typing import Hashable, Tuple +from typing import Any, Hashable, Sequence, Tuple + +import numbers + +import numpy as np TVec = Tuple[int, ...] NodeId = Hashable @@ -26,21 +30,53 @@ def zero_tvec(dim: int) -> TVec: return (0,) * dim -def validate_tvec(tvec: TVec, dim: int) -> None: - """Validate that `tvec` is an integer tuple of the correct dimension. +def validate_tvec(tvec: Any, dim: int) -> None: + """Validate that `tvec` is an integer translation vector + of dimension `dim`. + + Accepts: + - tuples/lists of integer-likes, + - 1D NumPy arrays, + - sequences containing NumPy integer scalars. Args: tvec: Translation vector. dim: Required dimension. Raises: - ValueError: If `tvec` has wrong length or non-int entries. + ValueError: If `tvec` has wrong length, wrong shape, or non-integer + entries. + + Notes: + - Entries must be instances of :class:`numbers.Integral`. + - ``bool`` is rejected (even though it is a subclass of ``int``). + - This function validates only; containers store translation vectors + internally as tuples of Python ``int``. """ - if len(tvec) != dim: - raise ValueError(f'tvec must have length {dim}, got {len(tvec)}') - for x in tvec: - if not isinstance(x, int): - raise ValueError('tvec entries must be int') + + if dim <= 0: + raise ValueError('dim must be positive') + + if isinstance(tvec, np.ndarray): + if tvec.ndim != 1: + raise ValueError('tvec must be a 1D sequence') + if tvec.shape[0] != dim: + raise ValueError( + f'tvec must have length {dim}, got {int(tvec.shape[0])}' + ) + seq: Sequence[Any] = tvec.tolist() + else: + try: + n = len(tvec) # type: ignore[arg-type] + except TypeError as e: + raise ValueError('tvec must be a sized sequence') from e + if n != dim: + raise ValueError(f'tvec must have length {dim}, got {n}') + seq = tvec # type: ignore[assignment] + + for x in seq: + if isinstance(x, bool) or not isinstance(x, numbers.Integral): + raise ValueError('tvec entries must be integer-like (Integral)') def add_tvec(a: TVec, b: TVec) -> TVec: diff --git a/src/pbcgraph/graph.py b/src/pbcgraph/graph.py index ccb0a92..e6055f7 100644 --- a/src/pbcgraph/graph.py +++ b/src/pbcgraph/graph.py @@ -27,6 +27,8 @@ from __future__ import annotations +from dataclasses import dataclass + from typing import ( TYPE_CHECKING, Any, @@ -38,6 +40,8 @@ Tuple, ) +from types import MappingProxyType + import networkx as nx if TYPE_CHECKING: @@ -45,6 +49,14 @@ from pbcgraph.alg.components import components as _components +from pbcgraph.core.ordering import ( + stable_sorted, + stable_tvec, + # stable_unique_sorted, + try_sort_edges, + try_sort_neighbor_edges, +) + from pbcgraph.core.types import ( EdgeKey, NodeId, @@ -52,6 +64,7 @@ TVec, add_tvec, neg_tvec, + sub_tvec, validate_tvec, ) @@ -60,17 +73,57 @@ _USER_ATTRS = '_attrs' -def _unique_in_order(items: Iterable[NodeId]) -> List[NodeId]: - """Deduplicate an iterable while preserving the order - of first occurrence.""" - seen: set[NodeId] = set() - out: List[NodeId] = [] - for x in items: - if x in seen: - continue - seen.add(x) - out.append(x) - return out +def _ro(mapping: Dict[str, Any]) -> MappingProxyType: + """Return a read-only live view of a mapping.""" + return MappingProxyType(mapping) + + +def _validate_edge_key(key: EdgeKey) -> None: + """Validate an edge key. + + Edge keys must be ints, but ``bool`` is rejected (even though it is a + subclass of ``int``). + """ + if isinstance(key, bool) or not isinstance(key, int): + raise TypeError('edge key must be an int (bool is not allowed)') + + +@dataclass(frozen=True) +class _UKey: + """Private directed-edge key for undirected containers. + + `PeriodicGraph` and `PeriodicMultiGraph` represent each undirected + edge as two directed realizations. When `u == v` (self-loop in the + quotient) these two realizations would collide in NetworkX if they + shared the same `(u, v, key)` triple. + + `_UKey` splits the user-visible *base* key into two internal keys + distinguished by `dir` in {+1, -1}. + + The public API always exposes the base key (an int). + """ + + base: int + dir: int + + def __post_init__(self) -> None: + if self.dir not in (-1, 1): + raise ValueError('dir must be +1 or -1') + + +def _base_key(k: object) -> int: + """Return the public base edge key for an internal key. + + Args: + k: An internal key, either an int (directed containers) or a + `_UKey` (undirected containers). + + Returns: + The user-visible base key as a Python int. + """ + if isinstance(k, _UKey): + return int(k.base) + return int(k) class PeriodicDiGraph: @@ -89,7 +142,7 @@ class PeriodicDiGraph: structural_version: Incremented when the quotient structure changes (nodes/edges added or removed). data_version: Incremented when user data changes without structural - changes (edge attribute updates). + changes (node/edge attribute updates). Notes: - Quotient nodes are `NodeId` values. @@ -103,7 +156,6 @@ def __init__(self, dim: int = 3): if self._dim <= 0: raise ValueError('dim must be positive') self._g: nx.MultiDiGraph = nx.MultiDiGraph() - self._next_key: int = 0 self.structural_version: int = 0 self.data_version: int = 0 @@ -142,8 +194,10 @@ def add_node(self, u: NodeId, **attrs: Any) -> None: Notes: - Increments `structural_version` if the node is new. - - If the node already exists, this only updates attributes - and increments `data_version` if any attributes are provided. + - If the node already exists and `attrs` are provided, + this updates attributes and increments `data_version`. + - If the node is new, attributes provided at creation do not + increment `data_version` (pure structural change semantics). """ exists = self._g.has_node(u) if not exists: @@ -151,7 +205,8 @@ def add_node(self, u: NodeId, **attrs: Any) -> None: self.structural_version += 1 if attrs: self._g.nodes[u].update(attrs) - self.data_version += 1 + if exists: + self.data_version += 1 def remove_node(self, u: NodeId) -> None: """Remove a quotient node and all incident edges. @@ -172,18 +227,22 @@ def has_node(self, u: NodeId) -> bool: return self._g.has_node(u) def nodes(self, data: bool = False) -> Iterable: - """Iterate quotient nodes. + """Iterate quotient nodes in deterministic order. Args: - data: If True, yield `(u, attrs)` where `attrs` is a live mapping. + data: If True, yield `(u, attrs)` where `attrs` is a read-only + live view of the node attribute mapping. Returns: Iterable of node ids or `(node, attrs)` pairs. """ - return self._g.nodes(data=data) + nodes = stable_sorted(self._g.nodes) + if not data: + return iter(nodes) + return ((u, _ro(self._g.nodes[u])) for u in nodes) - def get_node_data(self, u: NodeId) -> Dict[str, Any]: - """Return the live node attribute mapping. + def get_node_data(self, u: NodeId) -> MappingProxyType: + """Return a read-only live view of the node attribute mapping. Args: u: Node id. @@ -191,7 +250,7 @@ def get_node_data(self, u: NodeId) -> Dict[str, Any]: Raises: KeyError: If node is missing. """ - return self._g.nodes[u] + return _ro(self._g.nodes[u]) def set_node_attrs(self, u: NodeId, **attrs: Any) -> None: """Update node attributes and increment `data_version`. @@ -212,14 +271,36 @@ def set_node_attrs(self, u: NodeId, **attrs: Any) -> None: # ----------------- # Edges # ----------------- - def _fresh_key(self) -> int: - key = self._next_key - self._next_key += 1 - return key + def _alloc_key_directed(self, u: NodeId, v: NodeId) -> EdgeKey: + """Allocate a new edge key for a directed edge (u -> v). - def _maybe_advance_key_counter(self, key: EdgeKey) -> None: - if isinstance(key, int) and key >= self._next_key: - self._next_key = key + 1 + Mirrors NetworkX's ``new_edge_key`` behavior: start from + ``len(keys)``, then increment until unused. + """ + kd = self._g.get_edge_data(u, v) + if not kd: + return 0 + k = len(kd) + while k in kd: + k += 1 + return int(k) + + def _alloc_key_undirected(self, u: NodeId, v: NodeId) -> EdgeKey: + """Allocate a new edge key for an undirected edge between u and v. + + Keys are allocated in *public base-key* space. Undirected containers + store directed realizations using private `_UKey` objects, so the + internal MultiDiGraph keys are not necessarily ints. + """ + kd_uv = self._g.get_edge_data(u, v) or {} + kd_vu = self._g.get_edge_data(v, u) or {} + used = {_base_key(k) for k in kd_uv} | {_base_key(k) for k in kd_vu} + if not used: + return 0 + k = len(used) + while k in used: + k += 1 + return int(k) def _key_for_tvec( self, u: NodeId, v: NodeId, tvec: TVec @@ -240,10 +321,10 @@ def _key_for_tvec( adj = self._g.adj[u] if v not in adj: return None - want = tuple(tvec) + want = stable_tvec(tvec) for k, ed in adj[v].items(): if tuple(ed[_TVEC_ATTR]) == want: - return k + return _base_key(k) return None def _add_edge_impl( @@ -264,17 +345,18 @@ def _add_edge_impl( self.add_node(v) if key is None: - key = self._fresh_key() + key = self._alloc_key_directed(u, v) else: - self._maybe_advance_key_counter(key) + _validate_edge_key(key) # Disallow overwriting an existing directed edge id. if self._g.has_edge(u, v, key=key): raise KeyError((u, v, key)) + tvec_norm = stable_tvec(tvec) user_attrs: Dict[str, Any] = dict(attrs) self._g.add_edge( - u, v, key=key, **{_TVEC_ATTR: tuple(tvec), _USER_ATTRS: user_attrs} + u, v, key=key, **{_TVEC_ATTR: tvec_norm, _USER_ATTRS: user_attrs} ) self.structural_version += 1 return key @@ -336,12 +418,12 @@ def edge_tvec(self, u: NodeId, v: NodeId, key: EdgeKey) -> TVec: data = self._g.get_edge_data(u, v, key) if data is None: raise KeyError((u, v, key)) - return tuple(data[_TVEC_ATTR]) + return stable_tvec(data[_TVEC_ATTR]) def get_edge_data( self, u: NodeId, v: NodeId, key: EdgeKey, default: Any = None ) -> Any: - """Return the live user attribute mapping for an edge. + """Return a read-only live view of the user attribute mapping. Args: u: Source node id. @@ -350,12 +432,13 @@ def get_edge_data( default: Value to return if edge is missing. Returns: - The live user attribute dict, or `default` if missing. + A read-only live view of the user attribute mapping, or `default` + if missing. """ data = self._g.get_edge_data(u, v, key) if data is None: return default - return data[_USER_ATTRS] + return _ro(data[_USER_ATTRS]) def set_edge_attrs( self, u: NodeId, v: NodeId, key: EdgeKey, **attrs: Any @@ -379,30 +462,64 @@ def remove_edge(self, u: NodeId, v: NodeId, key: EdgeKey) -> None: self._g.remove_edge(u, v, key=key) self.structural_version += 1 - def edges(self, keys: bool = False, data: bool = False) -> Iterable: - """Iterate directed edges. + def edges( + self, keys: bool = False, data: bool = False, tvec: bool = False + ) -> Iterable: + """Iterate directed edges in deterministic order. Args: keys: If True, include the multiedge key. - data: If True, include the live user attribute mapping. + data: If True, include the read-only user attribute mapping. + tvec: If True, include the translation vector. Returns: - An iterable of `(u, v)` / `(u, v, key)` / `(u, v, attrs)` / - `(u, v, key, attrs)`. + An iterable of: + - `(u, v)` + - `(u, v, attrs)` + - `(u, v, key)` + - `(u, v, tvec)` + - `(u, v, tvec, key)` + - `(u, v, key, attrs)` + - `(u, v, tvec, attrs)` + - `(u, v, tvec, key, attrs)` """ - if data: - for u, v, k, edata in self._g.edges(keys=True, data=True): - attrs = edata[_USER_ATTRS] + records: List[Tuple[Any, Any, Tuple[int, ...], int, Any]] = [] + for u, v, k, edata in self._g.edges(keys=True, data=True): + records.append( + ( + u, v, + stable_tvec(edata[_TVEC_ATTR]), + _base_key(k), edata[_USER_ATTRS] + ) + ) + + try_sort_edges(records) + + for u, v, tv, k, attrs in records: + if data: + attrs_ro = _ro(attrs) + if tvec: if keys: - yield u, v, k, attrs + if data: + yield u, v, tv, k, attrs_ro + else: + yield u, v, tv, k else: - yield u, v, attrs - else: - for u, v, k in self._g.edges(keys=True, data=False): + if data: + yield u, v, tv, attrs_ro + else: + yield u, v, tv + else: if keys: - yield u, v, k + if data: + yield u, v, k, attrs_ro + else: + yield u, v, k else: - yield u, v + if data: + yield u, v, attrs_ro + else: + yield u, v # ----------------- # Neighborhoods @@ -422,23 +539,81 @@ def neighbors( if not self._g.has_node(u): raise KeyError(u) - def iter_edges() -> Iterator[ - Tuple[NodeId, EdgeKey, TVec, Dict[str, Any]] - ]: - adj = self._g.adj[u] - for v in adj: - kd = adj[v] - for k in kd: - ed = kd[k] - yield v, k, tuple(ed[_TVEC_ATTR]), ed[_USER_ATTRS] + records: List[Tuple[Any, Tuple[int, ...], int, Any]] = [] + adj = self._g.adj[u] + for v in adj: + kd = adj[v] + for k in kd: + ed = kd[k] + records.append( + ( + v, stable_tvec(ed[_TVEC_ATTR]), + _base_key(k), ed[_USER_ATTRS] + ) + ) + + try_sort_neighbor_edges(records) + + for v, tv, k, attrs in records: + if data: + attrs_ro = _ro(attrs) + if keys: + if data: + yield v, tv, k, attrs_ro + else: + yield v, tv, k + else: + if data: + yield v, tv, attrs_ro + else: + yield v, tv - if not keys and not data: - return ((v, t) for (v, _k, t, _a) in iter_edges()) - if keys and not data: - return ((v, t, k) for (v, k, t, _a) in iter_edges()) - if (not keys) and data: - return ((v, t, a) for (v, _k, t, a) in iter_edges()) - return ((v, t, k, a) for (v, k, t, a) in iter_edges()) + def in_neighbors( + self, u: NodeId, keys: bool = False, data: bool = False + ) -> Iterable: + """Iterate incoming periodic edges into quotient node `u`. + + The returned translation vector is the one stored on the directed edge + ``v -> u`` (i.e. *not* negated). + + Yields: + Depending on flags: + - `(v, tvec)` + - `(v, tvec, key)` + - `(v, tvec, attrs)` + - `(v, tvec, key, attrs)` + """ + if not self._g.has_node(u): + raise KeyError(u) + + records: List[Tuple[Any, Tuple[int, ...], int, Any]] = [] + pred_adj = self._g.pred[u] + for v in pred_adj: + kd = pred_adj[v] + for k in kd: + ed = kd[k] + records.append( + ( + v, stable_tvec(ed[_TVEC_ATTR]), + _base_key(k), ed[_USER_ATTRS] + ) + ) + + try_sort_neighbor_edges(records) + + for v, tv, k, attrs in records: + if data: + attrs_ro = _ro(attrs) + if keys: + if data: + yield v, tv, k, attrs_ro + else: + yield v, tv, k + else: + if data: + yield v, tv, attrs_ro + else: + yield v, tv def neighbors_inst( self, node_inst: NodeInst, keys: bool = False, data: bool = False @@ -458,9 +633,7 @@ def neighbors_inst( u, shift = node_inst validate_tvec(shift, self._dim) - def iter_lifted() -> Iterator[ - Tuple[NodeId, TVec, EdgeKey, Dict[str, Any]] - ]: + def iter_lifted() -> Iterator[Tuple[NodeId, TVec, EdgeKey, Any]]: for item in self.neighbors(u, keys=True, data=True): v, tvec, k, attrs = item yield v, add_tvec(shift, tvec), k, attrs @@ -473,27 +646,39 @@ def iter_lifted() -> Iterator[ return ((v, s2, a) for (v, s2, _k, a) in iter_lifted()) return ((v, s2, k, a) for (v, s2, k, a) in iter_lifted()) + def in_neighbors_inst( + self, node_inst: NodeInst, keys: bool = False, data: bool = False + ) -> Iterable: + """Iterate incoming lifted neighbors into a node instance. + + For an incoming edge ``v -> u`` with translation ``tvec``, the lifted + neighbor instance for ``v`` is ``shift - tvec``. + """ + u, shift = node_inst + validate_tvec(shift, self._dim) + + def iter_lifted() -> Iterator[Tuple[NodeId, TVec, EdgeKey, Any]]: + for item in self.in_neighbors(u, keys=True, data=True): + v, tvec, k, attrs = item + yield v, sub_tvec(shift, tvec), k, attrs + + if not keys and not data: + return ((v, s2) for (v, s2, _k, _a) in iter_lifted()) + if keys and not data: + return ((v, s2, k) for (v, s2, k, _a) in iter_lifted()) + if (not keys) and data: + return ((v, s2, a) for (v, s2, _k, a) in iter_lifted()) + return ((v, s2, k, a) for (v, s2, k, a) in iter_lifted()) + def successors(self, u: NodeId) -> Iterable[NodeId]: - """Return successor nodes (quotient) in insertion order - (duplicates removed).""" - return _unique_in_order( - v for (v, _t) in self.neighbors(u, keys=False, data=False) - ) + """Return successor nodes (quotient) in deterministic order.""" + vs = {v for (v, _t) in self.neighbors(u, keys=False, data=False)} + return stable_sorted(vs) def predecessors(self, u: NodeId) -> Iterable[NodeId]: - """Return predecessor nodes (quotient) in insertion order - (duplicates removed).""" - if not self._g.has_node(u): - raise KeyError(u) - preds: List[NodeId] = [] - seen: set[NodeId] = set() - pred_adj = self._g.pred[u] - for v in pred_adj: - if v in seen: - continue - seen.add(v) - preds.append(v) - return preds + """Return predecessor nodes (quotient) in deterministic order.""" + vs = {v for (v, _t) in self.in_neighbors(u, keys=False, data=False)} + return stable_sorted(vs) # ----------------- # Construction helpers @@ -593,7 +778,20 @@ class PeriodicGraph(PeriodicDiGraph): - ``u -> v`` with translation ``tvec`` - ``v -> u`` with translation ``-tvec`` - Both realizations share the same live user attribute mapping. + Both realizations share the same underlying user-attributes dict. + The public API returns read-only live views of that mapping. + + **Important:** a crystallographically common pattern is a quotient + self-loop with non-zero translation (``u == v`` and ``tvec != 0``), + representing a bond to a periodic image of the same motif. + + NetworkX identifies multiedges by ``(u, v, key)``. For self-loops, + the two directed realizations would collide if they shared the same key. + + To avoid this, `PeriodicGraph` stores directed realizations using a private + internal key type `_UKey(base, dir)`, where `base` is the user-visible + integer key and `dir` is `+1` / `-1`. The public API always exposes + the base key. In addition to the undirected-invariant pairing, this container enforces an invariant analogous to `PeriodicDiGraph`: @@ -615,6 +813,73 @@ def is_undirected(self) -> bool: by algorithms.""" return True + def edges( + self, keys: bool = False, data: bool = False, tvec: bool = False + ) -> Iterable: + """Iterate directed realizations in deterministic order. + + This iterator yields *directed realizations* of undirected edges. + + Note: + For self-loop periodic edges (``u == v`` and ``tvec != 0``), the + two directed realizations share the same ``(u, v, key)`` triple and + differ only by the translation vector. If `keys=True` but + `tvec=False`, this may yield duplicate ``(u, u, key)`` records. Use + `tvec=True` to disambiguate. + + See `PeriodicDiGraph.edges` for the record formats. + """ + return super().edges(keys=keys, data=data, tvec=tvec) + + def _internal_keys_for_base( + self, u: NodeId, v: NodeId, key: EdgeKey + ) -> List[object]: + """Return internal keys on (u -> v) whose public base key + equals `key`.""" + kd = self._g.get_edge_data(u, v) or {} + out: List[object] = [] + for ik in kd: + if isinstance(ik, _UKey): + if int(ik.base) == int(key): + out.append(ik) + else: + if int(ik) == int(key): + out.append(ik) + return out + + def _choose_internal_key( + self, u: NodeId, v: NodeId, key: EdgeKey + ) -> object: + """Choose a deterministic internal key for accessing + shared attrs/tvec.""" + keys = self._internal_keys_for_base(u, v, key) + if not keys: + raise KeyError((u, v, key)) + # Prefer the "forward" realization when present. + for ik in keys: + if isinstance(ik, _UKey) and ik.dir == 1: + return ik + # Otherwise choose a deterministic order. + return sorted( + keys, + key=lambda x: ( + 0 if isinstance(x, _UKey) else 1, + getattr(x, 'dir', 0), + repr(x), + ), + )[0] + + def _has_undirected_base(self, u: NodeId, v: NodeId, key: EdgeKey) -> bool: + """Return True if an undirected edge with base key exists + between u and v.""" + if u != v: + return ( + len(self._internal_keys_for_base(u, v, key)) == 1 and + len(self._internal_keys_for_base(v, u, key)) == 1 + ) + # Self-loop: both realizations live on (u -> u). + return len(self._internal_keys_for_base(u, u, key)) == 2 + def add_edge( self, u: NodeId, @@ -655,50 +920,158 @@ def _add_undirected_impl( self.add_node(v) if key is None: - key = self._fresh_key() + key = self._alloc_key_undirected(u, v) else: - self._maybe_advance_key_counter(key) + _validate_edge_key(key) - # Disallow overwriting existing keys in either direction. - if self._g.has_edge(u, v, key=key) or self._g.has_edge(v, u, key=key): + # Disallow overwriting an existing base key in either direction. + if ( + self._internal_keys_for_base(u, v, key) + or self._internal_keys_for_base(v, u, key) + ): raise KeyError((u, v, key)) + tvec_norm = stable_tvec(tvec) user_attrs: Dict[str, Any] = dict(attrs) + + k_fwd = _UKey(int(key), 1) + k_rev = _UKey(int(key), -1) + self._g.add_edge( u, v, - key=key, - **{_TVEC_ATTR: tuple(tvec), _USER_ATTRS: user_attrs}, + key=k_fwd, + **{_TVEC_ATTR: tvec_norm, _USER_ATTRS: user_attrs}, ) self._g.add_edge( v, u, - key=key, - **{_TVEC_ATTR: tuple(neg_tvec(tvec)), _USER_ATTRS: user_attrs}, + key=k_rev, + **{ + _TVEC_ATTR: stable_tvec(neg_tvec(tvec)), + _USER_ATTRS: user_attrs, + }, ) self.structural_version += 1 - return key + return int(key) def has_edge( self, u: NodeId, v: NodeId, key: Optional[EdgeKey] = None ) -> bool: if key is None: - return self._g.has_edge(u, v) and self._g.has_edge(v, u) - return ( - self._g.has_edge(u, v, key=key) and - self._g.has_edge(v, u, key=key) - ) + if u != v: + return self._g.has_edge(u, v) and self._g.has_edge(v, u) + kd = self._g.get_edge_data(u, u) or {} + return len(kd) >= 2 + return self._has_undirected_base(u, v, key) + + def edge_tvec(self, u: NodeId, v: NodeId, key: EdgeKey) -> TVec: + ik = self._choose_internal_key(u, v, key) + data = self._g.get_edge_data(u, v, ik) + if data is None: + raise KeyError((u, v, key)) + return stable_tvec(data[_TVEC_ATTR]) + + def get_edge_data( + self, u: NodeId, v: NodeId, key: EdgeKey, default: Any = None + ) -> Any: + try: + ik = self._choose_internal_key(u, v, key) + except KeyError: + return default + data = self._g.get_edge_data(u, v, ik) + if data is None: + return default + return _ro(data[_USER_ATTRS]) + + def set_edge_attrs( + self, u: NodeId, v: NodeId, key: EdgeKey, **attrs: Any + ) -> None: + if not self._has_undirected_base(u, v, key): + raise KeyError((u, v, key)) + if not attrs: + return + ik = self._choose_internal_key(u, v, key) + data = self._g.get_edge_data(u, v, ik) + if data is None: + raise KeyError((u, v, key)) + data[_USER_ATTRS].update(attrs) + self.data_version += 1 def remove_edge(self, u: NodeId, v: NodeId, key: EdgeKey) -> None: - if ( - not self._g.has_edge(u, v, key=key) - or not self._g.has_edge(v, u, key=key) - ): + triples = set() + for a, b in ((u, v), (v, u)): + kd = self._g.get_edge_data(a, b) or {} + for ik in kd: + if _base_key(ik) == int(key): + triples.add((a, b, ik)) + if len(triples) != 2: raise KeyError((u, v, key)) - self._g.remove_edge(u, v, key=key) - self._g.remove_edge(v, u, key=key) + for a, b, ik in triples: + self._g.remove_edge(a, b, key=ik) self.structural_version += 1 + def check_invariants(self, *, strict: bool = False) -> Dict[str, Any]: + """Check undirected pairing invariants. + + Returns a structured report and optionally raises on errors. + + Invariants checked: + - For every directed realization there is a paired reverse one. + - Translation vectors satisfy t(v,u,rev) = -t(u,v,fwd). + - The user-attributes dict is the *same object* for paired + realizations. + + Args: + strict: If True, raise ValueError on the first violation. + + Returns: + A dict with keys: `ok`, `errors`, `n_edges`. + """ + errors: List[str] = [] + + for u, v, ik, ed in self._g.edges(keys=True, data=True): + base = _base_key(ik) + if isinstance(ik, _UKey): + rev_key: object = _UKey(base, -ik.dir) + else: + rev_key = ik + rev = self._g.get_edge_data(v, u, rev_key) + if rev is None: + msg = ( + 'missing reverse edge for ' + f'({u!r}, {v!r}, base={base!r}, ik={ik!r})' + ) + if strict: + raise ValueError(msg) + errors.append(msg) + continue + tv = stable_tvec(ed[_TVEC_ATTR]) + tv_rev = stable_tvec(rev[_TVEC_ATTR]) + if tv_rev != stable_tvec(neg_tvec(tv)): + msg = ( + 'translation mismatch for paired edges: ' + f'({u!r}->{v!r}, base={base!r}) has {tv!r}, ' + f'({v!r}->{u!r}, base={base!r}) has {tv_rev!r}' + ) + if strict: + raise ValueError(msg) + errors.append(msg) + if ed[_USER_ATTRS] is not rev[_USER_ATTRS]: + msg = ( + 'attribute mapping is not shared for paired edges: ' + f'({u!r},{v!r}, base={base!r})' + ) + if strict: + raise ValueError(msg) + errors.append(msg) + + return { + 'ok': len(errors) == 0, + 'errors': errors, + 'n_edges': int(self._g.number_of_edges()), + } + class PeriodicMultiGraph(PeriodicGraph): """Undirected periodic multigraph. diff --git a/src/pbcgraph/lattice/snf.py b/src/pbcgraph/lattice/snf.py index d53ee6e..b93c85d 100644 --- a/src/pbcgraph/lattice/snf.py +++ b/src/pbcgraph/lattice/snf.py @@ -15,10 +15,9 @@ from __future__ import annotations from dataclasses import dataclass +from fractions import Fraction from typing import Iterable, List, Sequence, Tuple -import sympy as sp - from pbcgraph.core.types import TVec, validate_tvec @@ -288,7 +287,9 @@ class SNFDecomposition: Attributes: dim: Ambient lattice dimension `d`. rank: Rank of the sublattice `L` (number of non-zero diagonal entries). - diag: Full SNF diagonal (length `dim`), including 0 and 1 entries. + diag: Tuple of length `rank` with the (non-zero) SNF diagonal + entries (invariant factors). Empty tuple if there are no + generators. U: Unimodular change-of-basis matrix (left transform). U_inv: Inverse of `U` over the integers. """ @@ -317,6 +318,57 @@ def _to_tuple_matrix(A: List[List[int]]) -> Tuple[Tuple[int, ...], ...]: return tuple(tuple(int(v) for v in row) for row in A) +def _invert_unimodular_int_matrix(U: List[List[int]]) -> List[List[int]]: + """Invert a unimodular integer matrix exactly. + + The input matrix is assumed to have determinant +/-1, so the inverse is + an integer matrix. + """ + n = len(U) + if any(len(row) != n for row in U): + raise ValueError('matrix must be square') + + # Augment with identity and perform Gauss-Jordan elimination over Q. + A: List[List[Fraction]] = [ + [Fraction(int(U[i][j])) for j in range(n)] + + [Fraction(1 if i == j else 0) for j in range(n)] + for i in range(n) + ] + + for col in range(n): + pivot = None + for r in range(col, n): + if A[r][col] != 0: + pivot = r + break + if pivot is None: + raise ValueError('matrix is singular') + if pivot != col: + A[col], A[pivot] = A[pivot], A[col] + + piv = A[col][col] + A[col] = [x / piv for x in A[col]] + + for r in range(n): + if r == col: + continue + factor = A[r][col] + if factor == 0: + continue + A[r] = [x - factor * y for x, y in zip(A[r], A[col])] + + inv: List[List[int]] = [] + for i in range(n): + row_int: List[int] = [] + for j in range(n, 2 * n): + f = A[i][j] + if f.denominator != 1: + raise ValueError('unimodular inverse is not integral') + row_int.append(int(f.numerator)) + inv.append(row_int) + return inv + + def snf_decomposition( generators: Sequence[TVec], dim: int ) -> SNFDecomposition: @@ -361,10 +413,8 @@ def snf_decomposition( r += 1 diag.append(abs(di)) - # Compute inverse of U using SymPy (det is +/-1 so inverse is integer). - U_mat = sp.Matrix(U) - U_inv_mat = U_mat.inv() - U_inv = [[int(U_inv_mat[i, j]) for j in range(dim)] for i in range(dim)] + # U is unimodular (det = +/-1), so its inverse is integral. + U_inv = _invert_unimodular_int_matrix(U) return SNFDecomposition( dim=dim, diff --git a/tests/test_edges_tvec_flag.py b/tests/test_edges_tvec_flag.py new file mode 100644 index 0000000..147ed4f --- /dev/null +++ b/tests/test_edges_tvec_flag.py @@ -0,0 +1,20 @@ +from collections.abc import Mapping + +from pbcgraph import PeriodicMultiDiGraph + + +def test_edges_tvec_flag_shapes_and_order(): + G = PeriodicMultiDiGraph(dim=2) + k = G.add_edge('A', 'B', (1, 0), tag='x') + + assert list(G.edges(tvec=True)) == [('A', 'B', (1, 0))] + assert list(G.edges(keys=True, tvec=True, data=False)) == [ + ('A', 'B', (1, 0), k) + ] + + e = list(G.edges(keys=True, tvec=True, data=True)) + assert len(e) == 1 + u, v, tv, kk, attrs = e[0] + assert (u, v, tv, kk) == ('A', 'B', (1, 0), k) + assert isinstance(attrs, Mapping) + assert attrs['tag'] == 'x' diff --git a/tests/test_graph_basic.py b/tests/test_graph_basic.py index dc46529..15a9ac5 100644 --- a/tests/test_graph_basic.py +++ b/tests/test_graph_basic.py @@ -1,6 +1,6 @@ import pytest -from pbcgraph import PeriodicDiGraph, PeriodicGraph +from pbcgraph import PeriodicDiGraph, PeriodicGraph, PeriodicMultiDiGraph def test_add_edge_autocreates_nodes_and_stores_tvec(): @@ -52,6 +52,22 @@ def test_set_node_attrs_increments_data_version(): assert G.get_node_data('A')['foo'] == 1 +def test_add_node_with_attrs_data_version_semantics(): + G = PeriodicDiGraph(dim=1) + sv0 = G.structural_version + dv0 = G.data_version + + G.add_node('A', foo=1) + assert G.structural_version == sv0 + 1 + assert G.data_version == dv0 + assert G.get_node_data('A')['foo'] == 1 + + G.add_node('A', bar=2) + assert G.structural_version == sv0 + 1 + assert G.data_version == dv0 + 1 + assert G.get_node_data('A')['bar'] == 2 + + def test_periodicgraph_stores_two_directions_and_shared_attrs(): G = PeriodicGraph(dim=2) k = G.add_edge('A', 'B', (1, -1), kind='contact') @@ -64,7 +80,18 @@ def test_periodicgraph_stores_two_directions_and_shared_attrs(): attrs_ab = G.get_edge_data('A', 'B', k) attrs_ba = G.get_edge_data('B', 'A', k) - assert attrs_ab is attrs_ba + assert dict(attrs_ab) == dict(attrs_ba) + assert attrs_ab is not attrs_ba + + # Returned mappings are read-only views. + with pytest.raises(TypeError): + attrs_ab['x'] = 1 + + # Updates propagate because paired realizations share the same + # underlying user-attrs dict. + G.set_edge_attrs('A', 'B', k, foo=1) + assert G.get_edge_data('B', 'A', k)['foo'] == 1 + assert G.check_invariants()['ok'] is True assert attrs_ab['kind'] == 'contact' G.set_edge_attrs('A', 'B', k, strength=5) @@ -73,3 +100,9 @@ def test_periodicgraph_stores_two_directions_and_shared_attrs(): G.remove_edge('A', 'B', k) assert not G.has_edge('A', 'B', key=k) assert not G.has_edge('B', 'A', key=k) + + +def test_edge_key_rejects_bool(): + G = PeriodicMultiDiGraph(dim=1) + with pytest.raises(TypeError): + G.add_edge('A', 'B', (0,), key=True) diff --git a/tests/test_neighbors_inst.py b/tests/test_neighbors_inst.py index 0bc76c7..1258de0 100644 --- a/tests/test_neighbors_inst.py +++ b/tests/test_neighbors_inst.py @@ -1,3 +1,7 @@ +from collections.abc import Mapping + +import pytest + from pbcgraph import PeriodicDiGraph @@ -15,6 +19,9 @@ def test_neighbors_inst_lifts_translations(): assert ('C', (5, 4), k2) in out_k out_d = list(G.neighbors_inst(('A', (5, 5)), keys=False, data=True)) - # attrs are live dicts + # attrs are read-only live views for v, s2, attrs in out_d: - assert isinstance(attrs, dict) + assert isinstance(attrs, Mapping) + assert 'kind' in attrs + with pytest.raises(TypeError): + attrs['x'] = 1 diff --git a/tests/test_types.py b/tests/test_types.py index f61b377..7ee2617 100644 --- a/tests/test_types.py +++ b/tests/test_types.py @@ -1,3 +1,4 @@ +import numpy as np import pytest from pbcgraph import ( @@ -23,6 +24,15 @@ def test_validate_tvec_dim_and_int(): validate_tvec((0, 1, 2.0), 3) +def test_validate_tvec_numpy_and_rejects_bool(): + validate_tvec(np.array([0, 1, -2], dtype=np.int64), 3) + validate_tvec((np.int64(1), np.int32(0), np.int64(-1)), 3) + + # bool is an int subclass, but should be rejected + with pytest.raises(ValueError): + validate_tvec((True, 0, 0), 3) + + def test_tvec_arithmetic(): a = (1, -2, 3) b = (0, 5, -7) diff --git a/tests/test_undirected_self_loop.py b/tests/test_undirected_self_loop.py new file mode 100644 index 0000000..3deed79 --- /dev/null +++ b/tests/test_undirected_self_loop.py @@ -0,0 +1,78 @@ +import pytest + +from pbcgraph import PeriodicGraph, PeriodicMultiGraph + + +def test_periodicgraph_supports_self_loop_periodic_edge(): + G = PeriodicGraph(dim=1) + k = G.add_edge('A', 'A', (1,), kind='bond') + + # Stored as two directed realizations. + assert G.number_of_edges() == 2 + assert G.has_edge('A', 'A') is True + assert G.has_edge('A', 'A', key=k) is True + + edges = list(G.edges(keys=True, data=True, tvec=True)) + edge_quads = [(u, v, t, kk) for (u, v, t, kk, _a) in edges] + assert ('A', 'A', (1,), k) in edge_quads + assert ('A', 'A', (-1,), k) in edge_quads + + # Deterministic access: "forward" tvec is the one given in add_edge. + assert G.edge_tvec('A', 'A', k) == (1,) + + attrs = G.get_edge_data('A', 'A', k) + assert attrs['kind'] == 'bond' + with pytest.raises(TypeError): + attrs['x'] = 1 + + G.set_edge_attrs('A', 'A', k, strength=7) + assert G.get_edge_data('A', 'A', k)['strength'] == 7 + + # Instance neighbors show the infinite-lift semantics. + nbs = list(G.neighbors_inst(('A', (0,)), keys=True, data=False)) + assert ('A', (1,), k) in nbs + assert ('A', (-1,), k) in nbs + + comp = G.components()[0] + assert comp.rank == 1 + + rep = G.check_invariants() + assert rep['ok'] is True + + G.remove_edge('A', 'A', k) + assert G.number_of_edges() == 0 + assert G.has_edge('A', 'A') is False + + +def test_periodicgraph_self_loop_rejects_duplicate_tvec_up_to_reversal(): + G = PeriodicGraph(dim=1) + G.add_edge('A', 'A', (2,)) + with pytest.raises(ValueError): + G.add_edge('A', 'A', (2,)) + with pytest.raises(ValueError): + G.add_edge('A', 'A', (-2,)) + + +def test_periodicmultigraph_allows_parallel_self_loop_edges(): + G = PeriodicMultiGraph(dim=1) + k1 = G.add_edge('A', 'A', (1,), label='x') + k2 = G.add_edge('A', 'A', (1,), label='y') + + assert k1 != k2 + assert G.number_of_edges() == 4 + assert G.has_edge('A', 'A', key=k1) is True + assert G.has_edge('A', 'A', key=k2) is True + + edges = list(G.edges(keys=True, data=True, tvec=True)) + t1 = sorted([t for (_u, _v, t, k, _a) in edges if k == k1]) + t2 = sorted([t for (_u, _v, t, k, _a) in edges if k == k2]) + + assert t1 == [(-1,), (1,)] + assert t2 == [(-1,), (1,)] + + assert G.check_invariants()['ok'] is True + + G.remove_edge('A', 'A', k1) + assert G.number_of_edges() == 2 + assert G.has_edge('A', 'A', key=k1) is False + assert G.has_edge('A', 'A', key=k2) is True