#31680 closed enhancement (fixed)

Poset of manifold subsets

Reported by: Matthias Köppe Owned by:
Priority: major Milestone: sage-9.4
Component: manifolds Keywords:
Cc: Michael Jung, Eric Gourgoulhon, Travis Scrimshaw Merged in:
Authors: Matthias Koeppe Reviewers: Eric Gourgoulhon
Report Upstream: N/A Work issues:
Branch: 84896c4 (Commits, GitHub, GitLab) Commit: 84896c4d1ec02b22bed05d2f5dd188d71ee0767d
Dependencies: #31681, #31717 Stopgaps:

Status badges

Description

The declared subsets or supersets of a ManifoldSubset form finite posets. We add methods to expose them as instances of FinitePoset.

Change History (44)

comment:1 Changed 20 months ago by Michael Jung

Possibly related: #30263.

comment:2 Changed 20 months ago by Matthias Köppe

Yes, hopefully this new feature can be used as a debugging tool

comment:3 Changed 20 months ago by Matthias Köppe

Branch: u/mkoeppe/poset_of_manifold_subsets

comment:4 Changed 20 months ago by Matthias Köppe

Commit: 1b5c153e6c973c6fe0b2040dc2af2f12960b8716

Here's a first step: Building a digraph from the defined subsets.

(Plotting needs dot2tex and looks bad because of the long labels. I don't know if it is possible to replace the labels by the latex names)


New commits:

1b5c153ManifoldSubset.subset_digraph: New

comment:5 Changed 20 months ago by git

Commit: 1b5c153e6c973c6fe0b2040dc2af2f12960b871691bbc3ecb2b08dd3ffbb0ef1f3c2297de6b33816

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

91bbc3eManifoldSubset.subset_poset: New

comment:6 Changed 20 months ago by git

Commit: 91bbc3ecb2b08dd3ffbb0ef1f3c2297de6b33816858c82de2f280be41c6aa6c3928290c30c97f240

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

858c82dFixup

comment:7 Changed 20 months ago by Matthias Köppe

Dependencies: #31681

comment:8 Changed 20 months ago by git

Commit: 858c82de2f280be41c6aa6c3928290c30c97f240a1e21b0b4687f43f4f19b2c4ddac8e1e9493ae81

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

280a82dtrac #31681: fix sorting in layout_acyclic_dummy
a1e21b0Merge #31681

comment:9 Changed 20 months ago by Matthias Köppe

Authors: Matthias Koeppe

comment:10 Changed 20 months ago by git

Commit: a1e21b0b4687f43f4f19b2c4ddac8e1e9493ae812e2d91ebc69117b7a49e1f099f83993bfd118db8

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

2e2d91eManifoldSubset.{sub,super}set_{digraph,poset}: More options

comment:11 in reply to:  4 Changed 20 months ago by Eric Gourgoulhon

Replying to mkoeppe:

Here's a first step: Building a digraph from the defined subsets.

I'm happy to see this happening! At the beginning of the SageManifolds project, it was pretty clear that graphs would be the proper way to deal with subsets, but I was not familiar with this part of Sage and wanted to move fast...

(Plotting needs dot2tex and looks bad because of the long labels. I don't know if it is possible to replace the labels by the latex names)

This would be nice, indeed. Playing with the doctest examples,

sage: view(latex(D))                                                                                
sage: view(latex(P))                                                                                

yields nice figures, with the latex names.

Last edited 20 months ago by Eric Gourgoulhon (previous) (diff)

comment:12 Changed 20 months ago by Michael Jung

This digraph approach might also be useful to improve restrictions of tensor fields and make them even faster, no?

comment:13 in reply to:  12 ; Changed 20 months ago by Matthias Köppe

Replying to gh-mjungmath:

This digraph approach might also be useful to improve restrictions of tensor fields and make them even faster, no?

Yes, I think there are many more digraph / poset structures in sage.manifolds that could be exposed by similar methods in follow-up tickets.

On this ticket I am not making an attempt to actually change any of the internal representations of these structures. Just providing some tools that give a high-level view on the existing structures.

comment:14 in reply to:  13 Changed 20 months ago by Michael Jung

Replying to mkoeppe:

Replying to gh-mjungmath:

This digraph approach might also be useful to improve restrictions of tensor fields and make them even faster, no?

Yes, I think there are many more digraph / poset structures in sage.manifolds that could be exposed by similar methods in follow-up tickets.

On this ticket I am not making an attempt to actually change any of the internal representations of these structures. Just providing some tools that give a high-level view on the existing structures.

+1

Besides, I really like the way you write code. It's clear and neat!

Last edited 20 months ago by Michael Jung (previous) (diff)

comment:15 Changed 20 months ago by git

Commit: 2e2d91ebc69117b7a49e1f099f83993bfd118db802a82f90ccf15540fa8518e3591485092845d4d3

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

02a82f9Fix up handling of {lower,upper}_bound, add examples

comment:16 Changed 20 months ago by git

Commit: 02a82f90ccf15540fa8518e3591485092845d4d344c8d8c6f7cd8ed6b4055b464e82364e88cc9665

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

44c8d8cInclude example for open_covers=True

comment:17 Changed 20 months ago by Matthias Köppe

Status: newneeds_review

comment:18 Changed 20 months ago by git

Commit: 44c8d8c6f7cd8ed6b4055b464e82364e88cc96650571aaa5c5b52e0b7623476da8b7b3d10e3690ab

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

0571aaasrc/sage/manifolds/subset.py: Fix docstring markup

comment:19 Changed 20 months ago by Eric Gourgoulhon

Reviewers: Eric Gourgoulhon
Status: needs_reviewpositive_review

LGTM.

Side note: it's a pity that the subsets latex names cannot be used for the plots. I've not found such an option in the plot method of graphs; for posets, one can use the option element_labels as follows:

sage: M = Manifold(3, 'M')
sage: U = M.open_subset('U'); V = M.open_subset('V'); W = M.open_subset('W')
sage: VW = V.union(W)
sage: P = M.subset_poset()
sage: P.plot(element_labels={s: s._name for s in M.subsets()})

Do you think it is worth to include this trick in the documentation?

Last edited 20 months ago by Eric Gourgoulhon (previous) (diff)

comment:20 Changed 20 months ago by Matthias Köppe

Good idea, I'll add it. This is very useful.

comment:21 Changed 20 months ago by git

Commit: 0571aaa5c5b52e0b7623476da8b7b3d10e3690ab9d35006855c380d96d776e84ea8bab0caa93227f
Status: positive_reviewneeds_review

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

9d35006Relabel digraphs and posets for plotting

comment:22 Changed 20 months ago by Matthias Köppe

Also added examples for relabeled plotting of the digraphs.

comment:23 Changed 20 months ago by Matthias Köppe

Status: needs_reviewneeds_work

In light of #30263, I am going to revise the definition of the poset.

comment:24 Changed 20 months ago by git

Commit: 9d35006855c380d96d776e84ea8bab0caa93227f3ae1f9809350bf5d74c65c9020ae1fff4aa793d6

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

4d93fe5src/sage/manifolds/subset_set.py: New
3ae1f98ManifoldSubset.subset_digraph: Make subset nodes (singleton) ManifoldSubsetSet instances, open_cover nodes tuples

comment:25 Changed 20 months ago by Matthias Köppe

Status: needs_workneeds_review

Here's a new version in which the vertices of graph and poset representing manifold subsets are wrapped in an extra layer of a frozenset. This new format is intended to be future-proof for the changes necessary to fix #30263: Basically, one will need to quotient out by the equality relation of subsets.

In fact, instead of using frozenset directly, I define a subclass called ManifoldSubsetSet, which is convenient for printing and sorting.

comment:26 Changed 20 months ago by git

Commit: 3ae1f9809350bf5d74c65c9020ae1fff4aa793d6120fe97420a2ef8ebc914ad7c6c5393d1409cb8c

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

120fe97src/sage/manifolds/subset_set.py: Add documentation

comment:27 Changed 20 months ago by Matthias Köppe

This new class ManifoldSubsetSet could obviously be generalized to other finite sets of manifold objects. Would this be of interest?

comment:28 Changed 20 months ago by git

Commit: 120fe97420a2ef8ebc914ad7c6c5393d1409cb8c97ec61d80b555f6b5686987e9be58c366743a73b

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

97ec61dFiniteManifoldObjectFamily: Rename from ManifoldSubsetSet, subclass FiniteFamily instead of frozenset

comment:29 Changed 20 months ago by Matthias Köppe

Ready for review.

comment:30 Changed 20 months ago by Matthias Köppe

Dependencies: #31681#31681, #31717

comment:31 Changed 20 months ago by git

Commit: 97ec61d80b555f6b5686987e9be58c366743a73bf0959889a79f531474f8fe7a12ebe749db3c2957

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

cbbef90Adding a __bool__ to trivial family.
42abda7Adding __bool__ for other families.
1f21a8fMerge #31717
f095988FiniteManifoldObjectFamily.__bool__: Remove, inherited from superclass after #31717

comment:32 Changed 20 months ago by Eric Gourgoulhon

Thanks for the update. Looks good. I've got two remarks:

  1. Is there any reason for the names FiniteManifoldObjectFamily and FiniteManifoldSubsetFamily instead of ManifoldObjectFiniteFamily and ManifoldSubsetFiniteFamily? The latter ones sound more natural to me, making clear that we are dealing with finite families.
  1. To generate the documentation about these classes in the reference manual, the new entry sage/manifolds/family must be added to src/doc/en/reference/manifolds/manifolds.rst

comment:33 Changed 20 months ago by git

Commit: f0959889a79f531474f8fe7a12ebe749db3c29572f2ace2e9e9b0580d2b6d93eaa98d14bbc966f77

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

5d87ceeManifold{Object,Subset}FiniteFamily: Rename from FiniteManifold{Object,Subset}Family
2f2ace2src/doc/en/reference/manifolds/manifold.rst: Add sage.manifolds.family

comment:34 Changed 20 months ago by git

Commit: 2f2ace2e9e9b0580d2b6d93eaa98d14bbc966f771aff58a70849386432f336b5bba37e9545eed6eb

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

1aff58aFix up docstring markup

comment:35 in reply to:  32 Changed 20 months ago by Matthias Köppe

Replying to egourgoulhon:

  1. Is there any reason for the names FiniteManifoldObjectFamily and FiniteManifoldSubsetFamily instead of ManifoldObjectFiniteFamily and ManifoldSubsetFiniteFamily? The latter ones sound more natural to me, making clear that we are dealing with finite families.

Good idea, done.

  1. To generate the documentation about these classes in the reference manual, the new entry sage/manifolds/family must be added to src/doc/en/reference/manifolds/manifolds.rst

Thanks, done - I always forget to do this.

comment:36 Changed 20 months ago by git

Commit: 1aff58a70849386432f336b5bba37e9545eed6ebb922066d150a4bdc6e6ee9d52924a7d49f039946

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

b922066ManifoldSubsetFiniteFamily: If all subsets are open, include 'open' in repr

comment:37 Changed 20 months ago by Matthias Köppe

Did one final improvement for more informative printing

comment:38 Changed 20 months ago by git

Commit: b922066d150a4bdc6e6ee9d52924a7d49f03994630271af51d78e4796f1be8b221088ceb2a71b2fa

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

30271afFixup doctest

comment:39 Changed 20 months ago by Eric Gourgoulhon

Thanks for the changes. One minor remark: shouldn't

TypeError: all... subsets must have the same manifold

be replaced by

TypeError: all open subsets must have the same manifold

in the output of the doctest

sage: ManifoldSubsetFiniteFamily([M, N])

in ManifoldSubsetFiniteFamily ? Both doctests passed, but the latter is identical to the actual message get by the user.

Besides, do you think it is worth to add PLOT directives like

        .. PLOT::

            M = Manifold(3, 'M')
            U = M.open_subset('U'); V = M.open_subset('V'); W = M.open_subset('W')
            VW = V.union(W)
            P = M.subset_poset()
            g = P.plot(element_labels={element: element._name for element in P})
            sphinx_plot(g)

immediately after the doctest

            sage: P.plot(element_labels={element: element._name for element in P})

to generate plots of posets and digraphs in the reference manual?

In any case, I would suggest that the doctest markers # not tested are removed from the plot examples:

- sage: P.plot(element_labels={element: element._name for element in P})  # not tested
+ sage: P.plot(element_labels={element: element._name for element in P})                              
+ Graphics object consisting of 10 graphics primitives
Last edited 20 months ago by Eric Gourgoulhon (previous) (diff)

comment:40 Changed 20 months ago by git

Commit: 30271af51d78e4796f1be8b221088ceb2a71b2fa84896c4d1ec02b22bed05d2f5dd188d71ee0767d

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

215c378ManifoldSubset.subset_{digraph,poset}: Add plots in documentation, remove # not tested
84896c4ManifoldSubsetFiniteFamily: Use full error message in doctest

comment:41 Changed 20 months ago by Matthias Köppe

How's this look?

comment:42 in reply to:  41 Changed 20 months ago by Eric Gourgoulhon

Status: needs_reviewpositive_review

Replying to mkoeppe:

How's this look?

That's perfect, thanks!

comment:43 Changed 20 months ago by Matthias Köppe

Thanks for the review!

comment:44 Changed 18 months ago by Volker Braun

Branch: u/mkoeppe/poset_of_manifold_subsets84896c4d1ec02b22bed05d2f5dd188d71ee0767d
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.