Implement as_disjoint_union for unions of regions - #205
Open
Chessing234 wants to merge 3 commits into
Open
Conversation
try_union answers "do these polytopes have a convex union?", and every caller already handles None by keeping the pieces separate. Raising for an unbounded input therefore turns a routine "no" into a crash of whatever pipeline is simplifying regions -- and unbounded pieces are ordinary: subtracting a bounded domain from a half-space produces them. The vertex-only feasibility checks below still cannot see extreme rays, so the guard itself stays; only its answer changes.
Region.as_disjoint_union raised NotImplementedError for UNION, so any region built with Region.union could not be rewritten as a disjoint union -- the representation the zero-density and large-value code paths need before they can integrate or compare over a region. Each child piece is trimmed against the pieces already accepted, using Polytope.set_minus, which splits A \\ B into pairwise disjoint parts. The accumulated list therefore stays disjoint and its union is unchanged.
Three random rectangles per trial, so the overlaps are real: the pieces must contain exactly the points the union contains, and no two of them may share an interior point.
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
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.
What happens
Region.as_disjoint_unionis how a region gets rewritten into the form the restof the Python code can work with: a list of convex polytopes that cover it
without overlapping. For
Region_Type.UNIONit did not exist —so a region built with
Region.unioncould not be converted at all, and neithercould anything containing one.
Fix
Each child's pieces are trimmed against the pieces already accepted, with
Polytope.set_minus, which splitsA \ Binto pairwise disjoint parts. Theaccumulated list therefore stays disjoint at every step and its union is
unchanged, which is exactly the invariant the caller needs.
Subtracting a bounded piece from another leaves unbounded parts routinely, and
those reach
Polytope.try_unionthrough the simplification helpers, where theyhit the finite-polytope guard:
try_unionanswers "do these have a convex union?", and every caller alreadytreats
Noneas "no, keep them separate". Raising turns that routine answerinto a crash of the whole pipeline, so the guard now returns
None. Thevertex-only feasibility checks still cannot see extreme rays, so the check
itself stays.
Verification
tests/test_region.pygains a case in the style of the ones next to it: 50trials of three random rectangles, with real overlaps.
p.intersect(q).is_empty(include_boundary=False))Ran separately over random 3-dimensional boxes as well — 2400 sample points
across 6 configurations, no point covered zero times that should be covered, and
none covered twice.
tests/test_region.pypasses. Note thattests/test_all.pystill fails ontest_affine2's import oflarge_values, which is the breakage #200 fixes;this branch does not touch it.