Opened 6 months ago

Closed 3 months ago

#31688 closed enhancement (fixed)

Pullbacks (preimages) of manifold subsets under continuous maps

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.4
Component: manifolds Keywords:
Cc: egourgoulhon, tscrim, gh-mjungmath Merged in:
Authors: Matthias Koeppe Reviewers: Eric Gourgoulhon
Report Upstream: N/A Work issues:
Branch: 4558e26 (Commits, GitHub, GitLab) Commit: 4558e2634ad2f42d83d9a2930d7a5cd2f0310a7c
Dependencies: #31883, #31904, #31653, #31916, #31644, #31959, #31990, #21243 Stopgaps:

Status badges

Description (last modified by mkoeppe)

Similar to #31653, given a continuous map \Phi: N -> M and a manifold subset S of M, we define the pullback (preimage) of S as the subset of N of points p with \Phi(p) in S.

Given a real scalar field Phi: N -> R and a RealSet S, we define the pullback in the same way.

Also, we view a chart C as a continuous function Phi: C.domain() -> R^n and allow pulling back any subset of R^n (an object with a __contains__ method; for example polyhedra, lattices, linear subspaces, tensor modules) by it as well.

In all cases, because Phi is continuous, topological closures/interiors pull back.

An application is in #31981.

Change History (81)

comment:1 Changed 5 months ago by mkoeppe

  • Description modified (diff)

comment:2 Changed 5 months ago by mkoeppe

  • Dependencies set to #31883

comment:3 Changed 5 months ago by mkoeppe

  • Branch set to u/mkoeppe/pullbacks_of_manifold_subsets_under_continuous_maps

comment:4 Changed 5 months ago by git

  • Commit set to 941001ba6fc2aaceb78bdfcc9e92ada7bc5e3ebc

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

1942c71ScalarField.codomain: New, put scalar fields in category of continuous maps
8829d72Merge #31883
941001bManifoldSubsetPullback: More WIP

comment:5 Changed 5 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Description modified (diff)

comment:6 Changed 5 months ago by git

  • Commit changed from 941001ba6fc2aaceb78bdfcc9e92ada7bc5e3ebc to 90f763fa3afd8a7e8b1f856a3c44981a683602ca

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

90f763fManifoldSubsetPullback.__contains__: New

comment:7 Changed 5 months ago by git

  • Commit changed from 90f763fa3afd8a7e8b1f856a3c44981a683602ca to b8dadf3a3d4c4f8b6f4c9a84bdd65187789006f1

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

ff34765ManifoldSubsetPullback: In __classcall_private__, make immutable copies of the maps
b8dadf3ScalarField.pullback, ContinuousMap.pullback: New

comment:8 Changed 5 months ago by git

  • Commit changed from b8dadf3a3d4c4f8b6f4c9a84bdd65187789006f1 to 5abb86d0201bcb55a84cb9757043ba77a2837a03

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

5abb86dManifoldSubsetPullback: More WIP

comment:9 Changed 5 months ago by git

  • Commit changed from 5abb86d0201bcb55a84cb9757043ba77a2837a03 to e597e6d254b690ee1e5d02c703a4481e1b65ed5f

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

e597e6dManifoldSubsetPullback: Allow pulling back by charts, pulling back polyehdra

comment:10 Changed 5 months ago by mkoeppe

  • Description modified (diff)

comment:11 Changed 5 months ago by mkoeppe

  • Description modified (diff)

comment:12 Changed 5 months ago by git

  • Commit changed from e597e6d254b690ee1e5d02c703a4481e1b65ed5f to 1623cc5ed42df5ec4c430edbcca6874d47c38b70

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

1623cc5ManifoldSubsetPullback: Move computation of names to __classcall_private__

comment:13 Changed 5 months ago by git

  • Commit changed from 1623cc5ed42df5ec4c430edbcca6874d47c38b70 to dcc1f9a1b594a3b0a5756fd77bddfa624bf6ecaf

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

dcc1f9aManifoldSubsetPullback: Prepare for pullbacks of opens

comment:14 Changed 5 months ago by git

  • Commit changed from dcc1f9a1b594a3b0a5756fd77bddfa624bf6ecaf to a2dd1820ac0addb84f4dcc2fc3586d6ca88ed49f

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

a2dd182ManifoldSubsetPullback: Recognize more closed sets

comment:15 Changed 5 months ago by mkoeppe

  • Dependencies changed from #31883 to #31883, #31904
  • Summary changed from Pullbacks of manifold subsets under continuous maps to Pullbacks (preimages) of manifold subsets under continuous maps

comment:16 Changed 5 months ago by git

  • Commit changed from a2dd1820ac0addb84f4dcc2fc3586d6ca88ed49f to 79232db5b3cbc0e675a6257b2959d45ed78162ac

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

459758eImageManifoldSubset, ContinuousMap.image: Add optional 'inverse' argument, use it in __contains__
26c7e56src/sage/manifolds/continuous_map_image.py: Update doctests
3e273bbTopologicalSubmanifold.as_subset: New
9726d36Docstring work
19762aeImageManifoldSubset: New parameter domain_subset, use it in ContinuousMap.image
964f9f7src/sage/manifolds/continuous_map.py: Update copyright
e711215src/sage/manifolds/continuous_map_image.py: Add tests
0f3e36dLink in documentation of sage.manifolds.continuous_map_image
4a13d8bMerge #31653
79232dbContinuousMap.preimage: New, make pullback an alias; add example

comment:17 Changed 5 months ago by mkoeppe

  • Dependencies changed from #31883, #31904 to #31883, #31904, #31653
  • Description modified (diff)

comment:18 Changed 4 months ago by git

  • Commit changed from 79232db5b3cbc0e675a6257b2959d45ed78162ac to 3c6666a7415ecf500e8720e9e8a5015ce00d4d2a

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

3c6666aManifoldSubsetPullback: Make pullback of open RealSet under a ScalarField an open set

comment:19 Changed 4 months ago by git

  • Commit changed from 3c6666a7415ecf500e8720e9e8a5015ce00d4d2a to e95c897e4d2eac8e3488ac04a9b1e409f32f4e24

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

235d75aManifoldSubsetPullback.closure: Adjust doctest
aea4554#31904: Fix indentation in _pullback_chart
e95c897Merge #31904

comment:20 Changed 4 months ago by mkoeppe

  • Description modified (diff)

comment:21 Changed 4 months ago by git

  • Commit changed from e95c897e4d2eac8e3488ac04a9b1e409f32f4e24 to ff5eb8b4c870612c75f953df1021e4e9f627b9b2

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

ff5eb8bManifoldSubsetPullback: Add doctests

comment:22 Changed 4 months ago by git

  • Commit changed from ff5eb8b4c870612c75f953df1021e4e9f627b9b2 to c268d34a6e1fc726bd20293ecf2086b0711dc97b

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

c268d34ManifoldSubsetPullback: Fix example using the embedding map

comment:23 Changed 4 months ago by git

  • Commit changed from c268d34a6e1fc726bd20293ecf2086b0711dc97b to 2a01a070e2d2ab6efd35ab7afd1c25bd7b210527

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

56b22a3ManifoldSubsetPullback.__classcall_private__: Add doc
762e0a8ManifoldSubsetPullback.__init__: Add tests
2a01a07ManifoldSubsetPullback._an_element_: New

comment:24 Changed 4 months ago by mkoeppe

  • Status changed from new to needs_review

This is not quite ready yet, but comments are very welcome

comment:25 Changed 4 months ago by git

  • Commit changed from 2a01a070e2d2ab6efd35ab7afd1c25bd7b210527 to acc71c3127b9d70b72e96fc7e4de1a95fbdc6bfb

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

e9c670csage.geometry.polyhedron.relint, Polyhedron_base.relative_interior, Polyhedron_base.interior: New
86fc6cdMerge #31916
acc71c3ManifoldSubsetPullback: Pulling back interiors of polyhedra

comment:26 Changed 4 months ago by mkoeppe

  • Dependencies changed from #31883, #31904, #31653 to #31883, #31904, #31653, #31916

comment:27 Changed 4 months ago by gh-mjungmath

For a possible follow-up, it might be a good idea to bear in mind that the preimage of a regular value of a differentiable map between manifolds of dimension n and m respectively is a differentiable manifold of dimension n-m again.

comment:28 Changed 4 months ago by git

  • Commit changed from acc71c3127b9d70b72e96fc7e4de1a95fbdc6bfb to 3a5870e784ecda9fe611f2299b77fd86381bdd35

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

9df2104sage.geometry.relative_interior: Move here from sage.geometry.polyhedron.relint
b8bfe20ConvexRationalPolyhedralCone: Add methods interior, relative_interior
6869673relative_interior: Fix for dimension 0
021d073RelativeInterior: Add documentation, tests, comparison methods, method relative_interior
8f38e04ConvexRationalPolyhedralCone.interior, relative_interior: Add doctests
3a5870eMerge #31916

comment:29 Changed 4 months ago by mkoeppe

  • Description modified (diff)

comment:30 Changed 4 months ago by git

  • Commit changed from 3a5870e784ecda9fe611f2299b77fd86381bdd35 to e802a21ad48a2caba2808913e8971d9bf4bca504

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

d11280esrc/sage/manifolds/subsets/pullback.py: Fixup import
cd3ca79Update doctests for refined category of ScalarField
105bb8bMerge #31883
5f9c852RelativeInterior.interior: New
5c089ecRelativeInterior.__eq__, __ne__: Handle comparisons with objects of other types
86ce301Polyhedron_base.is_relatively_open: New; fix relative_interior for affine subspaces
216cb81ConvexRationalPolyhedralCone.is_relatively_open: New, fix ConvexRationalPolyhedralCone.relative_interior for linear subspaces
44cde1esrc/doc/en/reference/discrete_geometry/index.rst: Add sage/geometry/relative_interior
fa4c2d2Whitespace fixes
e802a21Merge #31916

comment:31 Changed 4 months ago by mkoeppe

  • Description modified (diff)

comment:32 Changed 4 months ago by mkoeppe

  • Dependencies changed from #31883, #31904, #31653, #31916 to #31883, #31904, #31653, #31916, #31959

comment:33 Changed 4 months ago by git

  • Commit changed from e802a21ad48a2caba2808913e8971d9bf4bca504 to 9ea0cea86fffe3946dec52970677ebb434d48a30

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

6bef52bConvexSet_base._test_convex_set: Run the testsuite of relint
6ab5677RelativeInterior.is_universe: New
c085d30Polyhedron_base.interior: Handle the empty polyhedron correctly
686d0afPolyhedron_base.product: Add doctest for alias 'cartesian_product'
2b1d108Merge #31919
7323b10ConvexSet_base._test_contains: Only test extension to AA for exact base rings
e0d2eebMerge #31959
e6c3ed5RelativeInterior.__hash__: New
4c7d57fsrc/sage/geometry/relative_interior.py: Fix doctest output
9ea0ceaManifoldSubsetPullback: Do not trip over relative interiors of cones

comment:34 Changed 4 months ago by egourgoulhon

As revealed by the patchbot, there are some doctest failures, as well as some coverage and pyflakes issues.

comment:35 Changed 4 months ago by git

  • Commit changed from 9ea0cea86fffe3946dec52970677ebb434d48a30 to d59123489ea39f9cda94f6e4dc45b35848fff8ea

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

94e6858RelativeInterior.ambient, ambient_vector_space, is_universe: New
0c9bc94ConvexSet_base: Add default implementations of ambient, ambient_dim; add doctests
d591234Merge #31959

comment:36 Changed 4 months ago by mkoeppe

  • Status changed from needs_review to needs_work

Thanks for taking a look! Yes, I'll have to add more documentation and examples.

comment:37 Changed 4 months ago by git

  • Commit changed from d59123489ea39f9cda94f6e4dc45b35848fff8ea to 87f3e302501cc5cbc565834ca7c77a3630784bc7

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

87f3e30ManifoldSubsetPullback: Add documentation and tests

comment:38 Changed 4 months ago by mkoeppe

  • Dependencies changed from #31883, #31904, #31653, #31916, #31959 to #31883, #31904, #31653, #31916, #31959, #31990
Last edited 4 months ago by mkoeppe (previous) (diff)

comment:39 Changed 4 months ago by git

  • Commit changed from 87f3e302501cc5cbc565834ca7c77a3630784bc7 to 754e002c2cbb79d43d24a5a8f6dafa06664e85cf

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

7d3ae5cMerge #31959
2b1040cConvexSet_base.{an_element,some_elements}, Polyhedron.some_elements, RelativeInterior.some_elements: New
a884517PolyhedronFace.some_elements: New
f6cc8f7Merge #31990
1a4805cManifoldSubsetPullback: For charts, use .point as the inverse
754e002ManifoldSubsetPullback._an_element_, some_elements: Implement

comment:40 Changed 4 months ago by git

  • Commit changed from 754e002c2cbb79d43d24a5a8f6dafa06664e85cf to a8e607cee6cad630ee7f2220cd36c07223f80d5f

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

a8e607cManifoldSubsetPullback._coord_def: Add documentation and examples

comment:41 Changed 4 months ago by mkoeppe

  • Status changed from needs_work to needs_review

comment:42 Changed 4 months ago by git

  • Commit changed from a8e607cee6cad630ee7f2220cd36c07223f80d5f to c49ab33b27112434199e45ade9dbd1b3c88ad2fe

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

ce91e44src/sage/geometry/relative_interior.py: Fix doctest output
b19b3ccMerge #31959
c49ab33Merge #31990

comment:43 Changed 4 months ago by git

  • Commit changed from c49ab33b27112434199e45ade9dbd1b3c88ad2fe to 7ae21daf6dc4984c36aed84e08b1a0e90cbbe44f

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

f0e7c58ambient_vector_space docstring: Fix bad blocks
200d967ConvexSet_base.ambient doctest: Actually test the method
1c2bef5Merge #31959
ed898e6ConvexSet_base.an_element, some_elements: Go through generator method _some_elements_, make some_elements return a list
7ae21daMerge #31990

comment:44 Changed 4 months ago by egourgoulhon

  • Reviewers set to Eric Gourgoulhon

Thanks for the last changes. Some comments regarding the documentation:

  • there is no entry for the class ManifoldSubsetPullback in the reference manual
  • in the files continuous_map.py and subsets/pullback.py, each instance of :class:`ManifoldSubset` should be replaced by :class:`~sage.manifolds.subset.ManifoldSubset`
  • the docstring of ScalarField.pullback should have an INPUT and OUTPUT sections similar to those of ContinuousMap.preimage. By the way, to be consistent with ContinuousMap.preimage, shouldn't ScalarField.pullback be renamed to ScalarField.preimage, with pullback defined as an alias to preimage?
  • the INPUT and OUTPUT sections of DiffMap.pullback should be updated to take into account the new arguments.

comment:45 Changed 4 months ago by mkoeppe

  • Dependencies changed from #31883, #31904, #31653, #31916, #31959, #31990 to #31883, #31904, #31653, #31916, #31644, #31959, #31990

comment:46 Changed 4 months ago by git

  • Commit changed from 7ae21daf6dc4984c36aed84e08b1a0e90cbbe44f to 3877c5ff4796060e21cab7ab7cec325db6926d46

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

8d3ad85Merge #31764
60505b0Merge #31798
b9909c0ManifoldSubsetClosure: Add examples, fix docstring markup
16c6a72ManifoldSubsetClosure, ManifoldSubset.closure: Improve documentation
2514586Merge #31644
3ca139asrc/doc/en/reference/manifolds: Add pullback
1d430eesrc/sage/manifolds/continuous_map.py, src/sage/manifolds/subsets: Use qualified class name in docstrings
1266de9src/sage/geometry/convex_set.py: Fix docstring markup
df64125ScalarField.preimage: Make pullback an alias; expand documentation
3877c5fManifoldSubsetPullback.is_closed: Remove unfinished NNC_Polyhedron example

comment:47 Changed 4 months ago by git

  • Commit changed from 3877c5ff4796060e21cab7ab7cec325db6926d46 to 60ea044447b0a07d63e5298820cab9fb3ecd2ffc

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

0c549f0Chart.pullback, preimage: New
7bb720eManifoldSubsetPullback.__contains__: Handle the case of charts, which return tuples of coordinates, not vectors
60ea044ContinuousMap.preimage, ScalarField.preimage: Document that output can also be a manifold

comment:48 Changed 4 months ago by mkoeppe

  • Dependencies changed from #31883, #31904, #31653, #31916, #31644, #31959, #31990 to #31883, #31904, #31653, #31916, #31644, #31959, #31990, #21243

comment:49 Changed 4 months ago by git

  • Commit changed from 60ea044447b0a07d63e5298820cab9fb3ecd2ffc to c36e43bf1b2cc5849e8df4a015ff2a8e1638d393

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

5b8cecaInternalRealInterval, RealSet: Add _latex_ methods
69ca854RealSet._repr_: Use unicode cup sign instead of +
dbdfc06InternalRealInterval, RealSet: Remove extra whitespace in latex, add documentation
a0e3bf5Merge #31880
181b128RealSet.is_open, is_closed, closure, interior, boundary: New
7f56338PiecewiseFunction: Adjust doctests for changed RealSet repr
8cd74b5Merge #31880
8fdb104RealSet.boundary: Add another doctest
9d41f0esrc/sage/sets/real_set.py: Remove unnecessary import
c36e43bMerge #21243

comment:50 follow-up: Changed 4 months ago by egourgoulhon

In the docstring of ManifoldSubsetPullback:

  • in the INPUT section, there seems to be no entry for ConvexSet_base in the documentation
  • in the following part of the EXAMPLES block:
    sage: r_squared = M.scalar_field(x^2+y^2)
    sage: r_squared.set_immutable()
    sage: cl_I = RealSet((1, 4)); cl_I
    (1, 4)
    sage: cl_O = ManifoldSubsetPullback(r_squared, None, I); cl_O
    
    I guess I in the last line (which, at this stage, is the imaginary number i) should be replaced by cl_I and the subsequent True and False should become False and True respectively. But then, cl_I and cl_O are identical to I and O in the example that follows...

comment:51 Changed 4 months ago by git

  • Commit changed from c36e43bf1b2cc5849e8df4a015ff2a8e1638d393 to c540d3532fc99cefe0896cc73075a8e0950c0c7d

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

c540d35ManifoldSubsetPullback: Fix up some doctests with closed intervals

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

Thanks for spotting this - I have fixed this and a similar example

comment:53 in reply to: ↑ 50 ; follow-up: Changed 4 months ago by mkoeppe

Replying to egourgoulhon:

In the docstring of ManifoldSubsetPullback:

  • in the INPUT section, there seems to be no entry for ConvexSet_base in the documentation

You may need to run ./sage -docbuild reference/discrete_geometry html

comment:54 in reply to: ↑ 53 Changed 4 months ago by egourgoulhon

Replying to mkoeppe:

Replying to egourgoulhon:

In the docstring of ManifoldSubsetPullback:

  • in the INPUT section, there seems to be no entry for ConvexSet_base in the documentation

You may need to run ./sage -docbuild reference/discrete_geometry html

Yes indeed!

comment:55 in reply to: ↑ 52 Changed 4 months ago by egourgoulhon

Replying to mkoeppe:

Thanks for spotting this - I have fixed this and a similar example

Thanks.

At the moment, we have

sage: M = Manifold(2, 'M')                                                                          
sage: X.<x,y> = M.chart()                                                                           
sage: M.identity_map().preimage(M)                                                                  
Subset Id_M_inv_M of the 2-dimensional differentiable manifold M
sage: M.identity_map().preimage(M) is M                                                             
False
sage: M.zero_scalar_field().preimage(RealSet.point(0))                                              
Subset zero_inv_{0} of the 2-dimensional differentiable manifold M
sage: M.zero_scalar_field().preimage(RealSet.point(0)) is M                                         
False

Do you think it would be easy to have both answers to be True? If no, this should not hamper the current ticket, but be differed to another ticket.

comment:56 Changed 4 months ago by git

  • Commit changed from c540d3532fc99cefe0896cc73075a8e0950c0c7d to facad9753750e64d39d5cc6087be292edff94beb

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

facad97ContinuousMap.preimage: Handle identity_map specially

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

Sure, that's easy to do and certainly a good idea. Here it is for ContinuousMap.preimage already, I'll look into the case of the scalar fields

comment:58 Changed 4 months ago by git

  • Commit changed from facad9753750e64d39d5cc6087be292edff94beb to c2ecf3ebd9d00b2fac2fe8e5d4aca0b3da1ea1ae

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

c2ecf3eScalarField.preimage: Handle the case of the zero scalar field

comment:59 Changed 4 months ago by mkoeppe

More general things such as detecting that the preimage of [0,oo) under squaring is the full domain will have to wait until a follow-up ticket

comment:60 in reply to: ↑ 57 ; follow-up: Changed 4 months ago by egourgoulhon

Replying to mkoeppe:

Sure, that's easy to do and certainly a good idea. Here it is for ContinuousMap.preimage already,

Thanks. Maybe one can add

if self._is_isomorphism and codomain_subset == self._codomain:
    return self._domain

comment:61 follow-up: Changed 4 months ago by egourgoulhon

The patchbot reports a doctest error in src/sage/geometry/polyhedron/base.py as well as pyflakes errors in src/sage/manifolds/subsets/pullback.py.

comment:62 in reply to: ↑ 60 ; follow-up: Changed 4 months ago by mkoeppe

Replying to egourgoulhon:

Maybe one can add

if self._is_isomorphism and codomain_subset == self._codomain:
    return self._domain

I think I can do more generally

  if self._codomain.is_subset(codomain_subset):
      return self._domain

comment:63 in reply to: ↑ 61 Changed 4 months ago by mkoeppe

Replying to egourgoulhon:

The patchbot reports a doctest error in src/sage/geometry/polyhedron/base.py

I've seen this one before locally, I don't think it's coming from this ticket, but I have opened #32030 for it

comment:64 Changed 4 months ago by git

  • Commit changed from c2ecf3ebd9d00b2fac2fe8e5d4aca0b3da1ea1ae to 3b1c4280d4b9d935e8b9fdcd7868343899ab2544

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

3b1c428ManifoldSubsetPullback: Make codomain_subset required 2nd init arg; fix pycodestyle/pyflakes warnings

comment:65 in reply to: ↑ 62 Changed 4 months ago by egourgoulhon

Replying to mkoeppe:

Replying to egourgoulhon:

Maybe one can add

if self._is_isomorphism and codomain_subset == self._codomain:
    return self._domain

I think I can do more generally

  if self._codomain.is_subset(codomain_subset):
      return self._domain

Yes, indeed! Being an isomorphism is not necessary in that case.

Last edited 4 months ago by egourgoulhon (previous) (diff)

comment:66 Changed 4 months ago by git

  • Commit changed from 3b1c4280d4b9d935e8b9fdcd7868343899ab2544 to d321b93a9a40effb0c67aae593fc6915e7fd9417

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

d321b93ContinuousMap: Return domain if the map's codomain is contained in the given subset

comment:67 Changed 4 months ago by egourgoulhon

A minor point: to generate correctly the documentation, the following change shall be made in the docstring of ManifoldSubsetPullback:

-    - ``map`` - an instance of :class:`ContinuousMap`,
+    - ``map`` - an instance of :class:`~sage.manifolds.continuous_map.ContinuousMap`,
       :class:`ScalarField`, or :class:`Chart`

For :class:`ScalarField` and :class:`Chart`, this is not necessary because these classes are imported in pullback.py.

comment:68 follow-up: Changed 4 months ago by egourgoulhon

This example in the doctstring of ManifoldSubsetPullback.is_closed is dubious:

        The pullback of finite sets is closed::

            sage: F = Family([vector(QQ, [1, 2], immutable=True), vector(QQ, [2, 3], immutable=True)])
            sage: McF = ManifoldSubsetPullback(c_cart, F, name='McF'); McF
            Subset McF of the 2-dimensional topological manifold R^2
            sage: McF.is_closed()
            False

comment:69 Changed 4 months ago by git

  • Commit changed from d321b93a9a40effb0c67aae593fc6915e7fd9417 to 653c651ca9477afdac82c2448b2507ffa85c1235

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

07aba9eManifoldSubsetPullback.is_closed: Preimage of finite sets is closed
653c651src/sage/manifolds/subsets/pullback.py: Fix docstring markup

comment:70 in reply to: ↑ 68 Changed 4 months ago by mkoeppe

Replying to egourgoulhon:

This example in the doctstring of ManifoldSubsetPullback.is_closed is dubious:

Indeed. Looks like I had forgotten to actually implement this case. Done now

comment:71 Changed 4 months ago by egourgoulhon

Thanks for the update. As soon as the patchbot visits the ticket and is green, the ticket can be set to positive review.

comment:72 follow-up: Changed 4 months ago by egourgoulhon

  • Status changed from needs_review to positive_review

OK, since the changes since the last time the patchbot visited the ticket are quite minor, let us move on without waiting for another visit.

Thank you for the new features introduced here!

comment:73 Changed 4 months ago by mkoeppe

Thanks for the review!

comment:74 in reply to: ↑ 72 Changed 4 months ago by egourgoulhon

Replying to egourgoulhon:

OK, since the changes since the last time the patchbot visited the ticket are quite minor, let us move on without waiting for another visit.

It seems that we don't have any patchbot at the moment: https://patchbot.sagemath.org/ticket/0/

comment:75 Changed 4 months ago by git

  • Commit changed from 653c651ca9477afdac82c2448b2507ffa85c1235 to 4b930ffd8509e1e9553d683787dad57ba9e81995
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

4b930ffMerge tag '9.4.beta4' into t/31688/pullbacks_of_manifold_subsets_under_continuous_maps

comment:76 Changed 4 months ago by mkoeppe

  • Status changed from needs_review to positive_review

comment:77 Changed 3 months ago by vbraun

  • Status changed from positive_review to needs_work
sage -t --long --warn-long 41.5 --random-seed=0 src/sage/geometry/polyhedron/base.py
**********************************************************************
File "src/sage/geometry/polyhedron/base.py", line 2975, in sage.geometry.polyhedron.base.Polyhedron_base.incidence_matrix
Failed example:
    M == P.incidence_matrix()
Expected:
    False
Got:
    True
**********************************************************************
1 item had failures:
   1 of  31 in sage.geometry.polyhedron.base.Polyhedron_base.incidence_matrix
    [1737 tests, 1 failure, 35.30 s]
----------------------------------------------------------------------
sage -t --long --warn-long 41.5 --random-seed=0 src/sage/geometry/polyhedron/base.py  # 1 doctest failed
----------------------------------------------------------------------

comment:78 Changed 3 months ago by git

  • Commit changed from 4b930ffd8509e1e9553d683787dad57ba9e81995 to 4558e2634ad2f42d83d9a2930d7a5cd2f0310a7c

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

4558e26Polyhedron_base.incidence_matrix: Remove accidental change to doctest

comment:79 Changed 3 months ago by mkoeppe

  • Status changed from needs_work to needs_review

comment:80 Changed 3 months ago by mkoeppe

  • Status changed from needs_review to positive_review

comment:81 Changed 3 months ago by vbraun

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