feat(math): add cubical complexes domain with f-vector and face closure (#1888) - #2020
feat(math): add cubical complexes domain with f-vector and face closure (#1888)#2020morluto wants to merge 1 commit into
Conversation
Create the cubical_complexes domain with two operations: - cubical.f_vector.compute: exact f-vector and Euler characteristic of a finite cubical complex composed of elementary unit lattice cubes. - cubical.face_closure.compute: complete face closure (all proper faces) of a set of elementary cubes, returning total cell count and cells by dimension. Partially addresses #1888.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
morluto
left a comment
There was a problem hiding this comment.
Review verdict: mathematically incorrect face model and f-vector
A cubical complex must be closed under both lower and upper faces. An elementary cube also needs degenerate intervals [a,a]; its dimension is the number of nondegenerate unit intervals, not the number of ambient coordinates. Sage's finite-cubical-complex documentation uses exactly this convention.
This implementation misses both facts:
CubicalCellrejects every degenerate interval, so it cannot represent vertices or lower-dimensional faces in a fixed ambient space.compute_face_closure()replaces selected intervals only by(a,a). It never creates the upper face(b,b).compute_f_vector()does not take face closure at all; it merely counts the supplied maximal cells bylen(intervals).
The included single-square test encodes the bug: a closed square has 4 vertices, 4 edges, and 1 square, so its closure has 9 cells and f-vector (4, 4, 1), not total 4 / (1, 2, 1). Four squares in a 2x2 grid have f-vector (9, 12, 4) and Euler characteristic 1; the current f_vector path reports (0, 0, 4) and Euler characteristic 4.
The representation should allow interval lengths in {0,1}, require a common ambient dimension, define cell dimension by the number of length-one factors, and generate each active factor's lower and upper degenerations. Please replace the current tests with these canonical counterexamples before merge.
Deep review summaryVerdict: REQUEST CHANGES — the face model and resulting f-vector are mathematically incorrect. A cubical complex is closed under both lower and upper faces. Elementary cubes also need degenerate intervals The implementation currently violates those requirements:
Canonical counterexamples:
Allow interval lengths in |
Summary
Create the
cubical_complexesdomain with two operations, partially addressing #1888.Operations
cubical.f_vector.compute— Compute the exact f-vector (cell counts by dimension) and Euler characteristic of a finite cubical complex composed of elementary unit lattice cubes.cubical.face_closure.compute— Compute the full face closure (all proper faces) of a set of elementary cubes, returning total cell count and cells by dimension.Tests
5 known-answer and adversarial tests covering:
Continue this on Linzumi