Opened 6 months ago

Closed 5 months ago

#31993 closed enhancement (fixed)

ConvexSet_base: Add affine_hull, affine_hull_projection, an_affine_basis

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.4
Component: geometry Keywords:
Cc: gh-kliem Merged in:
Authors: Matthias Koeppe Reviewers: Jonathan Kliem
Report Upstream: N/A Work issues:
Branch: b909bdd (Commits, GitHub, GitLab) Commit: b909bdd4aa5dc7927f523fafb3c402f1768b882a
Dependencies: #31963, #32013 Stopgaps:

Status badges

Description (last modified by mkoeppe)

We generalize an_affine_basis, affine_hull, affine_hull_projection, and AffineHullProjectionData (from #27366) from Polyhedron_base to ConvexSet_base.

To provide the default implementation of affine_hull_projection, this ticket also adds the Polyhedron_base methods linear_transformation and translation to the ConvexSet_base ABC, as well as dilation for completeness.

Change History (31)

comment:1 Changed 5 months ago by mkoeppe

  • Dependencies set to #31963

comment:2 Changed 5 months ago by mkoeppe

  • Branch set to u/mkoeppe/convexset_base__add_affine_hull__affine_hull_projection__an_affine_basis

comment:3 Changed 5 months ago by git

  • Commit set to 02269657e76342872e975cf99c25ace362e8939a

Branch pushed to git repo; I updated commit sha1. New commits:

2a96f18sage.geometry: Import cached_method from a more specific module
0226965WIP ConvexSet_base.affine_hull_projection

comment:4 follow-up: Changed 5 months ago by mkoeppe

ConvexRationalPolyhedralCone.solid_restriction is similar to affine_hull_projection

comment:5 Changed 5 months ago by git

  • Commit changed from 02269657e76342872e975cf99c25ace362e8939a to 23b08cdda64b8090bb07c8145df14e2215294435

Branch pushed to git repo; I updated commit sha1. New commits:

23b08cdRelativeInterior.an_affine_basis: New; relax _test_affine_basis to accept closure points

comment:6 in reply to: ↑ 4 Changed 5 months ago by mkoeppe

Replying to mkoeppe:

ConvexRationalPolyhedralCone.solid_restriction is similar to affine_hull_projection

... best taken care of in #30172, which adds a cone ABC

comment:7 Changed 5 months ago by git

  • Commit changed from 23b08cdda64b8090bb07c8145df14e2215294435 to 0e2965f082f7a886c3eee4dcb22e503b00608f2c

Branch pushed to git repo; I updated commit sha1. New commits:

0e2965fPolyhedron.affine_hull_projection: Refactor as ConvexSet_base.affine_hull_projection and Polyhedron._affine_hull_projection

comment:8 Changed 5 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Description modified (diff)
  • Status changed from new to needs_review

comment:9 Changed 5 months ago by git

  • Commit changed from 0e2965f082f7a886c3eee4dcb22e503b00608f2c to 837b7f5a88963083054c17de49fc6ee531e5256c

Branch pushed to git repo; I updated commit sha1. New commits:

837b7f5RelativeInterior.{dilation,linear_transformation,translation}: New

comment:10 Changed 5 months ago by mkoeppe

  • Description modified (diff)

comment:11 Changed 5 months ago by gh-kliem

Coverage needs work yet.

-geometry/relative_interior.py: 100.0% (14 of 14)
+geometry/relative_interior.py: 83.3% (15 of 18)

Probably three stupid tests, testing abstract methods raise a NotImplementedError.

comment:12 Changed 5 months ago by git

  • Commit changed from 837b7f5a88963083054c17de49fc6ee531e5256c to 535e4122ccc9a73c36c594889d52fe636b93b44b

Branch pushed to git repo; I updated commit sha1. New commits:

579303cConvexSet_base.dilation: Add default implementation, test
535e412ConvexSet_base.linear_transformation, translation: Add tests

comment:13 Changed 5 months ago by mkoeppe

  • Status changed from needs_review to needs_work

comment:14 Changed 5 months ago by git

  • Commit changed from 535e4122ccc9a73c36c594889d52fe636b93b44b to e0a1884daf8eaaa6c1e7b7301040236ade7a3540

Branch pushed to git repo; I updated commit sha1. New commits:

e0a1884ConvexSet_base._affine_hull_projection: Add doctest

comment:15 Changed 5 months ago by git

  • Commit changed from e0a1884daf8eaaa6c1e7b7301040236ade7a3540 to 9190a7150f3967c76a819fe31bd0bde6124b111b

Branch pushed to git repo; I updated commit sha1. New commits:

9190a71RelativeInterior: Add missing doctests

comment:16 Changed 5 months ago by mkoeppe

  • Status changed from needs_work to needs_review

comment:17 Changed 5 months ago by git

  • Commit changed from 9190a7150f3967c76a819fe31bd0bde6124b111b to 3a406bdb739e03220d78f1f90db05f80fc716b89

Branch pushed to git repo; I updated commit sha1. New commits:

3a406bdPolyhedron_base.an_affine_basis: Fix docstring markup

comment:18 Changed 5 months ago by mkoeppe

green patchbot, ready for review

comment:19 Changed 5 months ago by mkoeppe

  • Milestone changed from sage-9.4 to sage-9.5

comment:20 Changed 5 months ago by gh-kliem

Merge conflict.

comment:21 Changed 5 months ago by gh-kliem

  • Status changed from needs_review to needs_work

comment:22 Changed 5 months ago by git

  • Commit changed from 3a406bdb739e03220d78f1f90db05f80fc716b89 to a8d7cbd65b2b5d6962c329bb88b35d8505f0ce5f

Branch pushed to git repo; I updated commit sha1. New commits:

a8d7cbdMerge tag '9.4.beta5' into t/31993/convexset_base__add_affine_hull__affine_hull_projection__an_affine_basis

comment:23 Changed 5 months ago by mkoeppe

  • Milestone changed from sage-9.5 to sage-9.4
  • Status changed from needs_work to needs_review

Actually would be good to get it into 9.4 so that we don't have to bother with deprecation for the changes to AffineHullProjectionData

comment:24 Changed 5 months ago by gh-kliem

  • Reviewers set to Jonathan Kliem

I think things are fine.

Waiting for a patchbot, seems hopeless: https://patchbot.sagemath.org/ticket/0/

Coverage is good. I'm going to run sage -t --all on this ticket. Ping me, if I don't report in the next 24 hours.

comment:25 Changed 5 months ago by gh-kliem

I'm getting those failures

sage -t --random-seed=0 src/sage/misc/sagedoc.py  # 1 doctest failed
sage -t --random-seed=0 src/sage/tests/cmdline.py  # 5 doctests failed
sage -t --random-seed=0 src/sage/modular/local_comp/local_comp.py  # 1 doctest failed
sage -t --random-seed=0 src/sage/rings/number_field/number_field_ideal.py  # 1 doctest failed   

and nothing is related to this ticket, but I also get failures on develop.

comment:26 Changed 5 months ago by gh-kliem

  • Status changed from needs_review to positive_review

LGTM.

comment:27 Changed 5 months ago by mkoeppe

Thanks!

comment:28 Changed 5 months ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict

comment:29 Changed 5 months ago by git

  • Commit changed from a8d7cbd65b2b5d6962c329bb88b35d8505f0ce5f to b909bdd4aa5dc7927f523fafb3c402f1768b882a

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

bc26d4f{LatticePolytopeClass, ConvexRationalPolyhedralCone._some_elements_: New
e768463Merge #31877
cb93c99RealSet: Inherit from Set_base, Set_boolean_operators, Set_add_sub_operators
3a6f9bdRealSet.symmetric_difference: New
08c52c2Set_base._test_as_set_object: Skip _test_pickling
200b1efMerge tag '9.4.beta4' into t/32013/initialize_a_set_from_a_convexset_base_instance
63c2cfeConvexSet_base._test_convex_set: Do not test _test_as_set_object here
55240bbMerge #30473
fff2a79src/sage/sets/set.py: Fix docstring markup
b909bddMerge #32013

comment:30 Changed 5 months ago by mkoeppe

  • Dependencies changed from #31963 to #31963, #32013
  • Status changed from needs_work to positive_review

comment:31 Changed 5 months ago by vbraun

  • Branch changed from u/mkoeppe/convexset_base__add_affine_hull__affine_hull_projection__an_affine_basis to b909bdd4aa5dc7927f523fafb3c402f1768b882a
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.