Local ray casting over crease patterns, and open sink folds - #15
Open
imagirom wants to merge 31 commits into
Open
Local ray casting over crease patterns, and open sink folds#15imagirom wants to merge 31 commits into
imagirom wants to merge 31 commits into
Conversation
The vertex fan walk's loop bound `0 < theta < pi` is decided by rounding when the true angle lands on the boundary, which is reachable from generic input: in the 3-crossing grid fixture, arriving dead-centre of the SW 90-degree sector puts theta_2 at pi + 4.4e-16, and a 1-ulp nudge flips the crossing count from 1 to 3 and rotates d_out by 90 degrees. Per spec §1 a crease at folded angle theta is met at t = eps*cot(theta), so theta = pi means met at -infinity: not met. The strict bound is already mathematically right; only the float lands on the wrong side. Add `angle_tol=1e-9` and widen the exit rather than raising. The initial collinearity guard is a degenerate *input* and stays as-is. Also: - mirrored 3-crossing test for side="right", pinning `g = g.pre.rev` and the mirrored alternating sign (both previously unreached) - tests for the off-the-paper `face_out is None` path and the bad-`side` ValueError - imports moved to the top of tests/test_ray_casting.py - drop the gitignored spec path from the module docstring - correct the task-2 report: central_vertex() has no fixed position here, four vertices are tied and set iteration order picks the winner
…oundary
Mutation testing on task 4 found three branches no test could see.
- Vertex hits never occurred through cast_ray, so the fan branch, the
`p = vertex["pos"]` line and the distance-based vertex test were all
free to be wrong. Two tests now drive a ray onto a vertex, aimed a
deliberate 1e-12 off centre so the crossing parameter is snapped by
tolerance rather than by float luck.
- The heading half of _closes was untested; a self-intersection test and
a direct tolerance test now pin it in both directions.
- The border check consumed an iteration, so a path that finished within
max_steps was reported as "max_steps". It is now made before the loop
and after each step, and the boundary is pinned by a test.
Also: drop the unused G parameter of _walk, raise DegenerateRayError when
the ray sets off along its own start edge instead of silently picking a
side, and document all four stop reasons on RayPath and cast_ray -- with
the note that callers must test `not in ("closed", "border")`. RayHit's
docstring now describes what t really is at a fan.
Reversing the order of the backward pass's hits left each RayHit recording direction_in/direction_out/face in the sense the backward pass was travelling, so RayPath.hits read as two trajectories glued back to back rather than one. Splice re-oriented copies instead: directions negate and swap, and face shifts by one to the face entered travelling forward. Copies, not in-place mutation, so cast_ray stays idempotent. Also document "start" in cast_ray's Returns section, and re-anchor the end-ordering test on which half-edge each terminal hit crossed, since the face shift moves the off-paper None.
`_closes` compared `direction_in` against `d0`. The start hit does not
transmit, so its `direction_out == d0` is the direction the ray departed in;
returning to the start means arriving from the far side of the start edge and
transmitting across it, so the ray arrives on `transmit(d0, start_edge)` and
departs on `d0`. Only a perpendicular crossing makes the two the same -- which
the suite's one closure fixture happened to be, which is why nothing caught it.
Three Task 5 tests were asserting things that the bug made true: a fixture
described as a self-intersection is a genuine closed diamond, and two more had
caps that a now-closing ray reaches. Two Task 6 fixtures went stale. All
retargeted, none weakened.
A backward pass that closes while the forward one runs off the paper is now
common (226 of 1120 casts): the two half-rays leave into different faces, so
either can close alone. The result is a lasso, and `closed=False` with
`ends=("closed", ...)` already says exactly that -- documented, not changed.
The duplicate-edge guard in `add_ray_creases` stays: its periodic-`max_steps`
route is gone (68539 firings before, 6 after) but self-overlapping trajectories
still reach it, and disabling it corrupts 4-5 of 50 multi-ray patterns.
`_face_containing_segment`'s midpoint fallback stays too: 0 firings in 19617
segments, but nothing excludes the case.
The backward half of a both-ways cast set off along `-d`. The start point lies *on* the start edge `E`, so the trajectory crosses `E` there: it arrives along `transmit(d, E)` and departs along `d`, and retracing that arrival means leaving along `-transmit(d, E)`. Materializing makes the start point a degree-4 vertex, and only the transmitted heading makes it satisfy Kawasaki; `-d` kinks the rim at the one place it is guaranteed to cross a crease. It also makes the early return on a closed forward pass correct. Measured on 180 `rosette(7)` casts: all 24 forward-closing casts have a backward pass that closes on the identical set of segments, so a closed forward pass really has traced everything. Lassos -- one half looping while the other runs off the paper -- dropped to 0 of 12096 casts, as they should once the two halves are one line. `add_ray_creases` skipped a segment whose endpoints share no face, which happens when the ray crosses its own earlier path, and returned a rim with a gap in it; callers flood-fill bounded by the rim, so a gap leaks across the sheet. It now raises `DegenerateRayError`, both at the point of failure and on the finished rim. On the same sweep, gapped rims go from 40/180 to 0/180, with 65 rejected outright. A border-to-border rim is a path, so the rim is not required to close. Also: document the lasso case on `RayPath` rather than only in a comment; rename a test whose name contradicted its body; pin `_canonical` deterministically instead of only through a stochastic slow test.
`add_ray_creases` rejected a self-crossing ray in the middle of phase 2, where the missing common face shows up -- leaving the graph subdivided, part-creased and part-tagged, internally consistent so nothing noticed, and with nothing to identify and undo. The ray is cast to completion before anything is materialized, so a crossing is a property of the polyline through the hits alone. Test it there and raise before phase 1. Segments that merely share an endpoint touch rather than cross -- consecutive ones by construction, and the two ends of a closed path or of a lasso's cycle too -- so the filter is geometric, not index adjacency. Over a 180-cast rosette(7) sweep this rejects exactly the same 65 casts as before and leaves 0 graphs modified, against 65 before. Both existing checks stay as backstops. Their messages no longer render numpy scalars.
…both gates Every finding from review was coverage, not correctness. Five mutants survived the original suite: dropping the weak-minimality filter in _crimp_ok, min->max and gaps>tol->gaps>=0 in _cluster_margin, replacing _cluster_margin with return 0.0, and dropping either gate in local_assignment_valid. All are now killed. - unequal-sector _crimp_ok test: every rosette vertex has equal sectors, so 'crimp the smallest' was never exercised end-to-end - margin asserted exactly (pi/2 on the symmetric rosette) plus a skewed vertex with three clusters, so min and max no longer coincide - odd-degree and Kawasaki gates covered directly, since task 8 calls local_assignment_valid without the screening is_locally_flat_foldable does - thread tol from is_locally_flat_foldable into local_assignment_valid - measure the margin with the same eff_tol that decides the verdict - drop the unreachable negative-sector guard and its clamp, which also perturbed exact Kawasaki preservation - rename the tied-sector test to what it pins; the retry loop stays but is documented as untested insurance
A rim cast with both_ways=False has an untraced end (ends[0] == 'start') in
the middle of the sheet, so it does not separate the paper: the interior
flood fill leaks around it, every face comes out inside, and the sink
inverts the entire model. That is invisible to the flat-foldability check
because a global M/V flip preserves every local condition.
- accept only ('closed', 'border') as clean rim ends
- pass both_ways=True and raise TypeError if a caller tries to override it
- reset the rim to MOUNTAIN before raising InvalidSinkError, matching the
strict=False branch instead of leaving the last candidate tried
- document that there is no undo: sink into a copy of G to recover
- test that a sink into a genuinely flat-foldable pattern (shrink-rotate)
leaves the whole graph locally flat-foldable
- pin the deliberate MOUNTAIN-first tie-break on a symmetric rim
Add both modules to pleat/__init__.py's eager imports and to AGENTS.md, along with the flat_foldable.py entry that was missing entirely. Annotate the documented-but-unannotated parameters in both modules: griffe flagged five of them and mkdocs --strict aborted, which would have failed CI's docs job.
Critical: `t = 0` / `t = 1` are documented as meaning the start edge's endpoints, but the start hit is the one hit that never passes through `first_crossing`, so it carried the caller's raw parameter unsnapped. Phase 1 of `add_ray_creases` then subdivided an edge at a point already occupied: a duplicate vertex, a zero-length edge and two zero-area faces, on a graph that still passed `check_consistency`. Snapped in `_walk` rather than in the materializer, so `RayHit.vertex` is honest for every consumer and phase 1's own justification for keeping only the `orig` arm becomes true without exception. Also: - thread `angle_tol` through `_walk`, `cast_ray` and `add_ray_creases`, so both of the caster's tolerances are reachable from every public entry point; - rename the `"stalled"` end value, which used to be `"degenerate"` and read as an invitation to catch `DegenerateRayError` over a disjoint set of causes; - raise instead of returning `nan` / a silent `None` on a zero-length direction or crease, and render vertex positions rather than object reprs in the fan's error messages; - delete the unreachable graze branch in phase 1 and the lasso prose that the `transmit(-d, E)` fix made self-contradictory, and assert in `open_sink` that the impossible cycle-with-a-tail fails loudly; - keep `_face_containing_segment`'s midpoint disambiguation and the phase-1 sort key, with the reasoning and the deferred work they wait on written down; - run the geometry tests at a non-unit grid scale, so the six length-scalings in `first_crossing` and `_walk` stop being no-ops, and pin the clockwise arm of `_rim_is_ccw`, the self-crossing endpoint rule, and the endpoint starts; - add `pytest-timeout` and mark the caster's loop tests, so the two known regressions that present as hangs present as failures. Report: .superpowers/sdd/final-review-fix-report.md
…the start edge
`(halfedge, t)` with `t` at an endpoint is the documented way to start a ray at
a node, but `_walk` chose the face by which side of the start edge `direction`
pointed to. At a node of degree n that separates only the two sectors touching
that one edge; every other direction was sent into a face it pointed out of and
came back as a silent `ends[1] == "stalled"` after a single hit. Measured on a
degree-4 grid node, 3 of 8 headings worked.
`sector_at_vertex` is the departing counterpart to `fan_at_vertex`: the face
whose sector holds `d`, read off the incident half-edge that is the smallest
counter-clockwise turn away. `_walk` snaps the start to a vertex before
choosing the face and uses it there; strictly interior `t` keeps the
side-of-edge rule unchanged.
A heading along an incident crease at the start node is still degenerate, but
now raises one message ("sets off from <pos> along a crease") instead of two
that depended on whether the crease happened to be the half-edge passed.
`test_cast_ray_reports_a_stall_rather_than_calling_it_a_border` casts with a
`vertex_tol` wider than the grid, so its start snaps to a node and its due-east
heading became a crease heading; its direction is now off-axis.
`cast_ray`'s backward heading was `-transmit(d, E)` for every start, with `E` whichever incident half-edge the caller named the start with. At a node there is no canonical `E`, so a `both_ways` trace depended on that arbitrary choice: on a degree-4 node of the square grid the same direction gave two different trajectories depending on whether the naming edge ran vertically or horizontally. A ray passing exactly through a vertex is the degenerate case the fan rule exists for -- it passes an infinitesimal distance to one side, transmitting through a specific set of creases -- so the trajectory through the node is a fan passage, and retracing it is that passage reversed. The reversed ray arrives at `v` along `-d`, out of the sector holding `d`, and the fan hands back the heading and the face it leaves on. No start edge appears in that, hence edge-independence. At a degree-2 node -- a point that is effectively mid-edge -- the fan crosses the one other crease and gives exactly `-transmit(d, E)` back, so this is the same rule as before and not a second one; that reduction is pinned by test. The interior-`t` path is unchanged.
Covers cast_ray (transmission vs mirroring, the vertex fan, both_ways and node starts), add_ray_creases, open_sink with mountain/valley rendering before and after, the flat-foldability helpers, and the refusal cases. Added to the nav under a new "Manipulating crease patterns" group.
Traces the same rim as an open sink but assigns it differently. An open sink reverses every crease inside the rim and comes out uniform; a closed sink reverses only two -- the creases that end up outermost when the enclosed vertex is folded -- and switches the rim's assignment at every node except those two. The two exceptions are what make the loop close. Every rim node is degree 4: where the radial crease is untouched its two halves agree and contribute +-2, so the rim must switch for Maekawa; where it is reversed they cancel, so the rim must not. The rim therefore switches at n-2 of its n nodes, and n is even. Reversing all n closes the same loop the other way -- that is the open sink. The outermost pair is read off the folded angles psi, which are built from sector angles only and so never consult a crease assignment. Two things geometry does not fix are searched and validated instead: the rim's absolute value, and ties at the extremes of psi on a symmetric vertex. Validation runs at the sunk vertex as well as at every rim node -- the constraint there is real, since the new sum is sum(m) - 2(m_p + m_q). Limited for now to exactly one vertex inside a rim that is a closed cycle. That restriction is what makes every interior crease a radial half of the sunk vertex, which is what lets the rule be stated per crease at all.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a strictly local ray caster over the half-edge graph, and an open sink fold built on it.
What's here
pleat/ray_casting.py— cast a ray from a point on an edge through a crease pattern.At every crease it crosses the direction transmits:
keeping the component that crosses the crease and flipping the one along it. This is
deliberately not the mirror
2(d·û)û - d, which flips the crossing component and sends theray back into the face it came from. Checked against a square preliminary base: a rim segment
crossing the vertical book fold goes straight through, and crossing the 45° diagonal turns by
90° — together those trace the square rim of a square-base sink.
A ray that hits a vertex head-on is resolved as if it passed an infinitesimal distance to one
side. The epsilon cancels exactly, leaving a purely angular walk over the folded crease
positions — the same alternating prefix sum the flat-foldability test uses. The ray transmits
through however many creases the fan requires, which a single "pick one edge" rule gets wrong.
add_ray_creasesmaterialises a traced ray into the graph. A ray that would cross itselfis rejected before the graph is touched, because such a rim has a gap and the sink's flood fill
would leak through it.
Each step inspects only the face the ray is currently inside — no global geometry queries.
pleat/flat_foldable.py— a complete vertex-wise local flat-foldability test.Folded crease positions
ψare an alternating prefix sum whose last entry is the Kawasakisum, so Kawasaki is "the cycle closes" rather than a separate condition. Everything else
depends only on which
ψcoincide, so tolerance enters in exactly one place, andlocal_assignment_validreturns amarginalongside its verdict: a margin near the tolerancemeans the vertex is genuinely ambiguous, not that the answer is wrong. Validity itself is
Hull's crimp recursion with the big-little-big lemma.
is_locally_flat_foldableis now stricter — the crimp recursion replaces its Maekawa-onlybranch.
maekawa_checkstays public as a cheap necessary condition.pleat/sink.py—open_sink: trace a rim, invert every crease strictly inside it, andinfer whether the rim is mountain or valley. The rim is uniformly one or the other, so the
inference is a two-candidate test rather than a search. The degree-4 case falls out of the
general flat-foldability test instead of being special-cased.
docs/notebooks/Ray_Casting_and_Sinks.ipynb— a runnable walkthrough of all of it.Worth knowing
is_locally_flat_foldableis provably blind to a global mountain/valley flip: the assignmententers its verdict only through equality comparisons, so negating every assignment leaves the
verdict and the margin bit-identical. Every orientation-shaped error therefore produces exactly
a global complement and cannot be caught by an "is the result valid" assertion — only by
structural checks. The tests defend accordingly, and the design doc records it for future work.
Scope
Euclidean geometry only. Closed sink folds are not included. A ray aimed squarely at a
flat-foldable apex is unsupported (resolving it needs the holonomy of the full loop rather than
a local walk), and a self-crossing ray is rejected rather than split at its intersections.
789 tests pass,
blackclean,mkdocs build --strictclean.