moc xor + domain-bounded not#59
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #59 +/- ##
==========================================
+ Coverage 92.91% 93.19% +0.28%
==========================================
Files 25 25
Lines 3372 3498 +126
==========================================
+ Hits 3133 3260 +127
+ Misses 239 238 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
espg
left a comment
espg
left a comment
There was a problem hiding this comment.
🤖 from Claude (review)
Adversarial review of Phase 1 (xor) only — commit e361dec. The implementation is sound; the one real issue is a test that doesn't test what its comment claims.
Correctness — looks good. moc_xor mirrors moc_or/moc_and/moc_minus exactly (same build_bmoc → Bmoc::xor → bmoc_to_morton → normalize shape), and the empty-operand handling matches moc_or (a △ ∅ = a, ∅ △ ∅ = ∅). Answering Question (1) from the PR body: yes, that convention is internally consistent with the other verbs. Bmoc::xor exists in the pinned healpix 0.3.3, so it resolves. The Python wrapper and exports mirror the siblings 1:1.
MAX_DEPTH = 18 (PR Question (2)) — nothing broken for xor. xor shares the identical build_bmoc/cell_range path as the already-merged ops, so it inherits exactly the same depth-18 set-op ceiling — no regression introduced here, and raising it is correctly scoped out of #54.
Main finding — test_xor_order_zero_operand (Python) doesn't exercise the gap it documents. Its comment describes an operand "inside and outside that base cell" with "inside cells cancel," but it actually passes the fully-disjoint southern cover, so nothing is inside base cell 0 and nothing cancels. As written it's a duplicate of test_xor_disjoint_equals_or, and the #53 coverage gap is exercised only on the Rust side (test_xor_order_zero_operand in moc.rs, which does straddle base cell 0 via [5, 6, 300]). Inline comment has the fix options. This is the one I'd want addressed before merge.
Smaller, non-blocking:
- No
xortest sets bit 63 on a coarse (order-0) operand — the southern/bit-63 round-trip is covered forand/minuswith order-8 covers but not for a coarsexoroperand. Optional add. - The Rust order-0 test's
is_empty()assert is weak on its own, but the accompanyingref_xorequality already pins the exact result, so it's fine.
Test coverage is otherwise solid: brute-force equality, the xor == (a∪b)\(a∩b) identity, self/empty idempotence, disjoint==or, and xor folded into the 300-iteration mixed-order fuzz and the disjoint-cover test. Terse and convention-adherent throughout. No dependency, lint, or module-size (§4) concerns.
Generated by Claude Code
| def test_xor_order_zero_operand(self): | ||
| # An order-0 whole-base-cell operand against a finer cover that has cells | ||
| # inside and outside that base cell (the deferred #53 coverage gap). The | ||
| # inside cells cancel against the base cell's coverage; the outside cells | ||
| # survive. | ||
| base0 = mortie.norm2mort(0, 0, 0) # base cell 0 at order 0 | ||
| a = np.array([base0], dtype=np.uint64) | ||
| b = self._cover(self.S_LATS, self.S_LONS) # southern → a different base cell | ||
| got = set(int(x) for x in mortie.moc_xor(a, b)) | ||
| assert got == self._ref(a, b, 8, lambda x, y: x ^ y) | ||
| # b is wholly outside base cell 0, so xor == or == base0 ∪ b. | ||
| np.testing.assert_array_equal(mortie.moc_xor(a, b), mortie.moc_or(a, b)) |
There was a problem hiding this comment.
🤖 from Claude (review)
The comment here contradicts what the test actually does, and the test doesn't exercise the case it claims to.
The header comment (lines 728-731) says this is "a finer cover that has cells inside and outside that base cell ... The inside cells cancel against the base cell's coverage; the outside cells survive." But b is self._cover(self.S_LATS, self.S_LONS) — the southern square, which line 734 itself notes is "a different base cell," and line 737 then correctly states "b is wholly outside base cell 0, so xor == or." So b has no cells inside base cell 0: nothing cancels, and the inside-cancellation path described in the comment is never reached.
Two consequences:
- The descriptive comment is inaccurate — it reads like it was copied from the Rust
test_xor_order_zero_operand, which does construct an operand with cells both inside and outside base cell 0 ([5, 6, 300]at depth 4, with 5/6 inside and 300 outside). The Python twin lost that property. - As written this is functionally a duplicate of
test_xor_disjoint_equals_or(1) above — both assertmoc_xor == moc_oron a fully-disjoint pair, the only difference being thatais an order-0 single base cell rather than an order-8 cover. So the MOC boolean set algebra via the patched healpix BMOC fork #53 coverage gap ("coarse order-0 operand whose interior partially overlaps a finer operand") is genuinely exercised only on the Rust side, not in Python.
Suggest either (a) make b actually straddle base cell 0 (e.g. a cover with leaves inside base cell 0 plus some outside) so the cancellation path is tested at the Python boundary too, or (b) if you intend this to stay a disjoint order-0 case, fix the comment to say so and note that the inside-overlap path is covered by the Rust test — but then it adds little over (1).
Generated by Claude Code
| pub fn moc_xor(a: &[u64], b: &[u64]) -> Vec<u64> { | ||
| if a.is_empty() { | ||
| return normalize(b); | ||
| } | ||
| if b.is_empty() { | ||
| return normalize(a); | ||
| } | ||
| bmoc_to_morton(build_bmoc(a).xor(&build_bmoc(b))) | ||
| } |
There was a problem hiding this comment.
🤖 from Claude (review)
Implementation looks correct and mirrors moc_or/moc_and/moc_minus faithfully — same decode → op → encode → normalize shape via build_bmoc/bmoc_to_morton, and the empty-operand handling matches moc_or exactly (a △ ∅ = normalize(a), ∅ △ b = normalize(b), ∅ △ ∅ = ∅, since normalize(&[]) is empty). Bmoc::xor is present in the pinned healpix 0.3.3 (bmoc/ops.rs), so the call resolves.
One observation, not a blocker: unlike moc_minus (whose doc-comment calls out the upstream mixed-order infinite-loop fix in the pinned fork), there's no note here on whether xor was exercised against that same upstream mixed-order hazard. The 300-iteration mixed-order fuzz at the bottom of the module now covers xor, so this is fine — just flagging that the asymmetry in the doc-comments is intentional.
Generated by Claude Code
| // 300 is in another base cell (outside). | ||
| let base0 = nested2mort(0, 0); | ||
| let a = vec![base0]; | ||
| let b: Vec<u64> = [5u64, 6, 300].iter().map(|&n| nested2mort(n, 4)).collect(); | ||
| // Reference at depth 4 (the deepest operand cell). | ||
| assert_eq!(moc_xor(&a, &b), ref_xor(&a, &b, 4)); | ||
| // The two inside cells cancel against base0's coverage; 300 (outside) | ||
| // survives, so the result is non-empty. |
There was a problem hiding this comment.
🤖 from Claude (review)
This is the Rust test that genuinely exercises the deferred #53 gap, and it does so correctly: at depth 4 a base cell spans 4^4 = 256 = 1<<8 nested hashes, so 5 and 6 are inside base cell 0 and 300 is in base cell 1 (outside). The ref_xor brute-force assert pins the inside-cancel-plus-outside-survives behavior, and the non-empty assert guards against a vacuous pass.
Two strengthenings worth considering (non-blocking):
- The non-empty assert only proves something survived. Adding an assert that the result equals just
300's coverage (i.e.5/6fully cancelled and nothing of base cell 0 leaked) would pin the cancellation more tightly thanref_xorequality alone already does — thoughref_xorequality is in fact sufficient, so this is optional. - Consider also a southern (bit-63) order-0 operand, e.g. base cell 8 at order 0, to confirm the coarse-operand path round-trips the hemisphere prefix the same way the merged
test_southern_hemispheredoes forand/minus. Currently noxortest sets bit 63 on a coarse operand.
Generated by Claude Code
Merging this PR will not alter performance
Comparing 
Footnotes
|
|
🤖 from Claude Phase 2 (
The two optional Rust suggestions left standing: a southern/bit-63 coarse operand case is already covered for Phase 2 self-review running next. Generated by Claude Code |
espg
left a comment
espg
left a comment
There was a problem hiding this comment.
🤖 from Claude (review)
Phase-2 adversarial review of moc_not + _whole_sphere (commit eb4cbb5). No blocking defects. The core semantics are correct and I verified them against a fresh build (maturin develop --release, the 7 new/reworked tests pass):
moc_not(cover, domain) == moc_minus(domain, cover)✓ (matches @espg's locked decision and the "Python orchestration overmoc_minus" approval).- Default domain == the 12 order-0 base cells under the post-#58 packed-u64 encoding ✓ — built via
norm2mort, bit 63 set on bases 7–11, matches the internal helper and a hand-checked 12-cell set. - Out-of-domain → warn + clip, documented in the docstring
Warnsblock ✓;pytest.warns(..., match="outside")actually asserts the warning. - No circular-import risk:
from .tools import norm2mortis safe —tools.pyimports only_healpix,_rustie, and numpy, nevercoverage;__init__importscoveragebeforetoolsbutcoveragepullstoolsdirectly, so the order resolves. coverage.pyis 443 lines, well under the ~1000 limit (§4). flake8 E9/F-class clean. Scalarcoverand scalardomainboth work vianp.asarray(...).ravel(). Double-complement idempotence holds forcover ⊆ domain; the round-trip test correctly lands oncover ∩ domain(I confirmed it'scompress(inside), notcompress(cover), whencoverspills out).
Non-blocking findings (inline):
- Empty-domain emits a spurious warning —
moc_not(cover, domain=∅)warns "cells outside domain" for any non-empty cover, though the result is correctly∅. Defensible but misleading; no test pins the empty-domain path. - Warn predicate cost / wording — the extra
moc_minus(cover, domain)is discarded (PR question (3) already flags it); separately, a cover cell coarser than and straddling the domain boundary is flagged "outside" even though it's partly inside. test_default_domain_is_whole_sphereis near-tautological — it asserts the verbatim implementation (moc_not(cover) == moc_minus(whole_sphere, cover));test_complement_partitions_domainis the real guarantee. Consider pinning an actual expected complement instead.- Missing the hard clip case — out-of-domain test uses disjoint whole base cells; the straddling (coarse-cover-cell) clip, where clipping does non-trivial work, is untested.
- Style —
import warningslocalized in one test method while the sibling uses module-levelpytest.warns; move to file top for consistency (§4).
Nothing here blocks advancing. (1) and (4) are the two I'd most want addressed before this goes ready-for-review.
Generated by Claude Code
| # only ever subtracts the in-domain part of `cover`). | ||
| if moc_minus(cover, domain).size > 0: | ||
| warnings.warn( | ||
| "moc_not: `cover` has cells outside `domain`; they cannot be " |
There was a problem hiding this comment.
🤖 from Claude (review)
Spurious warning when domain is empty. With domain=∅, the out-of-domain predicate moc_minus(cover, domain).size > 0 is true for any non-empty cover, so moc_not(cover, domain=np.array([], dtype=np.uint64)) warns "cover has cells outside domain" — even though, semantically, an empty domain trivially has everything outside it and the (correct) result is ∅. Confirmed empirically: result is [], but a UserWarning fires.
It's defensible (the cells genuinely aren't in the domain), but the message implies user error where there is none. Worth either special-casing domain.size == 0 (return empty, no warn) or noting the empty-domain behavior in the docstring. Either way there's no test pinning the empty-domain path — see my note on the test class.
Generated by Claude Code
|
|
||
| # Cells of `cover` outside `domain` cannot be complemented within it; warn | ||
| # and clip them (the clip is implicit in `moc_minus(domain, cover)`, which | ||
| # only ever subtracts the in-domain part of `cover`). | ||
| if moc_minus(cover, domain).size > 0: |
There was a problem hiding this comment.
🤖 from Claude (review)
The warn predicate computes a full extra moc_minus(cover, domain) purely to decide whether to warn, then discards it and computes moc_minus(domain, cover) for the result — two BMOC round-trips where the docstring's own framing ("the clip is implicit in moc_minus(domain, cover)") says only the second is load-bearing. PR question (3) already flags the cost; covers are small so it's cheap, fine to keep. Two smaller notes if you do keep it:
- The predicate flags a
covercell that is coarser than and straddles thedomainboundary as "outside" (e.g.domain= one order-4 cell,cover= its order-0 parent → warns), even though part of that cover cell lies inside the domain. That's a reasonable reading of "outside", but the warning text ("cells outside domain") reads as if whole cells are wholly outside. Confirmed via partial-overlap probe. - Comment nit: line 434's "the clip is implicit in
moc_minus(domain, cover)" duplicates the docstringWarnsblock almost verbatim; one of the two is enough.
Generated by Claude Code
| def test_default_domain_is_whole_sphere(self): | ||
| # moc_not(cover) complements within the 12 order-0 base cells. | ||
| cover = self._cover(self.A_LATS, self.A_LONS) | ||
| got = mortie.moc_not(cover) | ||
| expected = mortie.moc_minus(self._whole_sphere(), cover) | ||
| np.testing.assert_array_equal(got, expected) |
There was a problem hiding this comment.
🤖 from Claude (review)
This test is near-tautological: it asserts moc_not(cover) == moc_minus(whole_sphere, cover), which is verbatim the implementation of moc_not (line 443: return moc_minus(domain, cover)) with the default-domain branch. It pins the wiring (default domain == 12 base cells, no transformation) but proves nothing about the result against an independent ground truth — if moc_minus and the default-domain build were both wrong in the same direction, this passes.
test_complement_partitions_domain is the stronger guarantee (union == domain, intersection == ∅) and isn't tautological. Consider either dropping this one or strengthening it to pin an actual expected complement (e.g. for a known small cover, assert the exact survivor base cells), so the default-domain encoding is checked against a value rather than against moc_minus itself.
Generated by Claude Code
| # complement is the finer cells not yet enumerated within the shard. | ||
| shard = np.atleast_1d(mortie.norm2mort(0, 0, 0)) # base cell 0 @ order 0 | ||
| enumerated = mortie.norm2mort([5, 6, 7], [0, 0, 0], 4) # inside the shard | ||
| import warnings |
There was a problem hiding this comment.
🤖 from Claude (review)
Style: import warnings inline in the method body, while the sibling test_out_of_domain_warns_and_clips uses the module-level pytest.warns. Move the import to the top of the file (it's stdlib, no reason to localize it) and/or use pytest.warns + pytest.warns(None)-style assertion consistently across the class. Minor, but it's the kind of inconsistency §4 ("match the surrounding code") calls out.
Generated by Claude Code
| def test_out_of_domain_warns_and_clips(self): | ||
| # cover with a cell outside the domain: warn, and clip to the domain. | ||
| shard = np.atleast_1d(mortie.norm2mort(0, 0, 0)) # base cell 0 | ||
| inside = mortie.norm2mort([5, 6], [0, 0], 4) | ||
| outside = np.atleast_1d(mortie.norm2mort([7], [5], 4)) # base 5 — outside | ||
| cover = np.concatenate([np.atleast_1d(inside), outside]) | ||
| with pytest.warns(UserWarning, match="outside"): | ||
| got = mortie.moc_not(cover, domain=shard) | ||
| # the clip makes it equal to complementing only the in-domain part. | ||
| np.testing.assert_array_equal(got, mortie.moc_minus(shard, inside)) |
There was a problem hiding this comment.
🤖 from Claude (review)
The warn-and-clip path is genuinely asserted (pytest.warns(UserWarning, match="outside")) — good, this is the strongest test in the class. Two coverage gaps in the same spirit, since this is the only "tricky" case:
inside(base 0) andoutside(base 5) are disjoint whole base cells, so the clip is the easy case. The harder one — acovercell coarser thandomainthat straddles the boundary (partially in, partially out) — is untested. That's the case where "clip away the out-of-domain part" actually does non-trivial work, and where the warn predicate's behavior is least obvious.- The clip assertion compares to
moc_minus(shard, inside), i.e. it re-derives the expected value from the sameinsidearray; it confirms the out-of-domain cell was dropped but, liketest_default_domain_is_whole_sphere, leans onmoc_minusrather than a pinned literal. Fine here givenpytest.warnscarries the real assertion.
Generated by Claude Code
|
🤖 from Claude Folded the Phase-2 self-review's diff-scoped findings (
Left standing (non-blocking, judged correct as-is): the warn predicate's extra Both phases are complete and CI is green (test 3.10/3.11/3.12, ruff, CodSpeed, wheels). Final local run: Generated by Claude Code |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
I'd like an example jupyter notebook that documents these new moc / bmoc operations that we've enabled -- both here and in #53 . This notebook should be binder runable (i.e., on binder.org); this may require updating the repo (i.e., requirements.txt, environment.yml, etc) so that repo2docker (which is what binder uses) can run cleanly against our repo. You should tell me explicitly what updates you decided to make this work, and these updates should not infer with our existing packaging / publishing pipeline via toml. For the notebook doc to be useful, it should have some narrative markdown describing what the operation (xor , not, etc) does for the morton indices, some example code showing it's use, and also some plot that visualize what is happening (i.e., before and after the operator). Feel free to ask any questions if you have them. |
espg
left a comment
espg
left a comment
There was a problem hiding this comment.
🤖 from Claude (review)
Adversarial fresh-context review of Phase 3 (6bd5993) — the docs notebook + Binder config. This phase is solid; the originating ask is met. Findings are two non-blocking robustness notes (inline); nothing requires a code change to land.
Pipeline untouched (the hard requirement). Confirmed git diff 6bd5993~1 6bd5993 -- pyproject.toml Cargo.toml .github/ environment.yml is empty — the change is purely additive (binder/environment.yml, binder/postBuild, examples/morton_set_algebra.ipynb, one README badge line). The "repo2docker reads only binder/ when present" isolation claim in the environment.yml header is correct, so the root dev env and the maturin/PyPI publish pipeline are not in repo2docker's path.
Build sanity. binder/postBuild is committed mode 100755 with a correct #!/bin/bash shebang and set -euo pipefail; python -m pip install . will compile the abi3/pyo3 extension using rustc/cargo from conda-forge rust on PATH. No stray artifacts in the commit (no *.so, .venv/, *~, target/).
Notebook correctness (read every cell).
- Uses the current public API only —
mort2polygon,mort2healpix,morton_coverage,moc_or/and/minus/xor/not. Verified against source:mort2polygonreturns[lat, lon]rows (the helper'scolumn_stack([latlon[:,1], latlon[:,0]])→ (lon,lat) is right),mort2healpixreturns(cell_id, order)(unpacked as_, ordercorrectly),morton_coverage(lats, lons, order=...)arg order matches. No stalelen(str(abs(int(m))))-1decimal-digit order inference anywhere. - All set-op orders are 5 — well under the order-18 BMOC ceiling, so no
ValueError. - Both identity asserts are valid against the implementation:
xor == or minus and, andnot(a, domain) == minus(domain, a)(confirmedmoc_notis literallymoc_minus(domain, cover)incoverage.py:448). The whole-sphere default +UserWarning-on-clip narrative matchescoverage.py:428–448. - Before/after plots present (
show_optwo-panel) for every verb; narrative descriptions are accurate. - Order-ceiling note is correct and matches reality (point path 29, set-ops 18) and links live issues #60 / #61 (both open, both about exactly this cap).
- Badge/
labpathURLs (mybinder.org/v2/gh/espg/mortie/HEAD?labpath=examples%2Fmorton_set_algebra.ipynb) are well-formed and consistent across README and the notebook's own header.
Conventions. Dependency-light by design (numpy + matplotlib only); no runtime dep added to pyproject.toml; terse and reviewable. Good.
Could not execute the notebook in this review env (no numpy installed here), so the assertions/plots were verified by reading the kernel against the source rather than by running — worth a clean Binder build before clearing draft.
Generated by Claude Code
| # the example notebooks import the in-tree version. Build isolation pulls the | ||
| # maturin build backend; the Rust toolchain comes from binder/environment.yml. | ||
| set -euo pipefail | ||
| python -m pip install . |
There was a problem hiding this comment.
🤖 from Claude (review)
python -m pip install . is correct and the abi3/pyo3 build will find rustc/cargo from the conda-forge rust package on PATH. One robustness note (non-blocking): pip's default build isolation ignores the maturin you pinned in environment.yml and fetches the maturin backend fresh from PyPI into the isolated build env. That's the normal maturin flow and works, but it means (a) the maturin>=1.0,<2.0 pin in environment.yml is not what actually builds the wheel — the build-system requires in pyproject.toml is — and (b) it adds a PyPI fetch at build time. If you want the conda-pinned maturin to be authoritative and skip the network fetch, python -m pip install --no-build-isolation . would do it (maturin + a Rust toolchain are both already in the env). Either is fine for Binder; flagging so the env pin isn't mistaken for the source of truth.
Generated by Claude Code
| channels: | ||
| - conda-forge | ||
| dependencies: | ||
| - python=3.11 |
There was a problem hiding this comment.
🤖 from Claude (review)
The notebook imports mortie in a Jupyter kernel, but the env has no jupyter/notebook/jupyterlab (and the README badge uses labpath=, i.e. JupyterLab). repo2docker auto-installs a Jupyter stack when none is declared, so this will still launch — but pinning it implicitly leaves the kernel/lab versions to repo2docker's defaults. Consider adding jupyterlab (or at least notebook) to make the labpath launch self-contained and reproducible. Non-blocking since repo2docker backfills it.
Generated by Claude Code
|
🤖 from Claude Phase 3 ( What's in it
Repo updates I made (and why) — explicitly, per your askEverything is purely additive; I changed nothing in
How it was testedBuilt the extension in a venv ( Self-review fold (
|
2 participants
Refs #54. Adds the two remaining BMOC-backed set-algebra verbs from the #54 follow-up — symmetric difference (
xor) and domain-bounded complement (not) — per the approved plan and @espg's decisions on that thread. Unblocked now that #58 (the packed-u64 migration) has landed, so the order-0 whole-sphere domain is pinned to the post-migration encoding.One commit per phase, title-only messages.
Phases
xor(symmetric difference)not(domain-bounded complement)Phase 1 —
xorMirrors the merged
moc_or/moc_and/moc_minusexactly (src_rust/src/moc.rs):src_rust/src/moc.rs:moc_xor(a, b) = bmoc_to_morton(build_bmoc(a).xor(&build_bmoc(b))), with the same empty-operand handling asmoc_or(a △ ∅ = a). Thehealpix0.3.3 BMOC exposesxoron theBmoctrait, so this is the identical decode → op → encode → normalize shape as the other three verbs.src_rust/src/lib.rs:rust_moc_xorPyO3 binding (wraps the compute inpy.allow_threads), registered in_rustie.mortie/coverage.py:moc_xor(a, b)Python wrapper; exported frommortie/__init__.py.moc.rs):ref_xorbrute-force harness (op = |x, y| x ^ y) mirroringref_or/ref_and/ref_minus; dedicatedtest_xor_brute_force,test_xor_self_and_empty, andtest_xor_order_zero_operand(an order-0 whole-base-cell operand against a finer cover with cells[5, 6, 300]@4— 5/6 inside base cell 0, 300 outside — the deferred #53 self-review gap; asserts the exact survivor set after densification);xorfolded into the disjoint-cover test and the 300-iteration mixed-order fuzz.test_coverage.py::TestMOCSetOps):test_xor_brute_force,test_xor_equals_or_minus_and,test_xor_self_and_empty,test_xor_disjoint_equals_or, andtest_xor_order_zero_operand(now exercises genuine partial overlap at the Python boundary — see self-review fold below).Phase 2 —
not(domain-bounded complement)Per the decisions on #54:
not(cover, domain) ≡ moc_minus(domain, cover); default domain = the 12 order-0 base cells. Pure-Python orchestration incoverage.pyover the mergedmoc_minus— no new Rust primitive.mortie/coverage.py:moc_not(cover, domain=None); exported frommortie/__init__.py.domain is None→ the 12 order-0 base cells, built vianorm2mort(zeros(12), arange(12), 0)(a_whole_sphere()helper) so it tracks the post-packed-u64 migration (Option A) + norm2mort consolidation #58 packed-u64 order-0 encoding rather than a hand-rolled constant.domaingiven → a single morton index or list/array (the "shard" cell case).covercells are clipped and warned (UserWarning), documented in the docstring'sWarnssection: the clip is implicit inmoc_minus(domain, cover), which only subtracts the in-domain part ofcover.test_coverage.py::TestMOCNot): whole-sphere default, complement-partitions-domain (cover ∪ not == domain,cover ∩ not == ∅), shard case (no warning), double-complement round-trip (not(not(cover)) == cover ∩ domain), out-of-domain warn-and-clip (pytest.warns), and empty-cover-returns-domain.How it was tested
Built in the worktree venv (
maturin develop --release):The 8 skips are pre-existing/environmental (shapely/parquet/coarse-cell/issue-#20 placeholder), unchanged by this PR.
Questions for review
xorempty-operand semantics.a △ ∅returnsnormalize(a)(and∅ △ ∅ = ∅), matchingmoc_or. Confirm that's the intended convention.MAX_DEPTH = 18inmoc.rs. Both new verbs inherit the same BMOCMAX_DEPTH = 18range-linearization ceiling as the existing ops — not raised to the kernel's order-29 here. Raising the MOC ceiling to 29 is out of scope for Follow-ups from the BMOC set-algebra PR (#53) #54; flag if you want it tracked separately.moc_nothome. Kept in Python overmoc_minusper your approval. The out-of-domain detection costs one extramoc_minus(cover, domain)purely to decide whether to warn; covers are small so this is cheap, but flag if you'd rather skip the warning check.