Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make ppl mutable #32158

Closed
kliem opened this issue Jul 7, 2021 · 20 comments
Closed

Make ppl mutable #32158

kliem opened this issue Jul 7, 2021 · 20 comments

Comments

@kliem
Copy link
Contributor

kliem commented Jul 7, 2021

We allow for ppl polyhedra to be mutable.

This is a preparation for allowing interactive modification. At the moment not much can be done, but to delay the calculation of Vrepresentation and Hrepresentation.

The goal is that normaliz and polymake use Polyhedron_interactive as well and allow adding generators/inequalities.

Depends on #32157

CC: @jplab @mkoeppe @yuan-zhou

Component: geometry

Keywords: ppl, lazy double description

Author: Jonathan Kliem

Branch/Commit: fce9f98

Reviewer: Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/32158

@kliem kliem added this to the sage-9.4 milestone Jul 7, 2021
@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 7, 2021

comment:2

How about "mutable" instead of "interactive"

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 7, 2021

comment:3

And surely by default, polyhedra should remain immutable, right?

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 7, 2021

comment:4

IMO, the constructor Polyhedron should get an additional keyword argument immutable=True (unfortunately there's a bit of inconsistency - in the Sage library one finds all three of immutable=, is_mutable=, is_immutable=).
When False, it should select a suitable backend

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 7, 2021

comment:5

See also #29101 for my proposed "Refined protocol for _element_constructor_ in element classes with mutability"

@kliem
Copy link
Contributor Author

kliem commented Jul 8, 2021

comment:6

Thanks for the comments. Yes, using immutable is much better. I would prefer to not have another backend though. It's the same with vectors and matrices. One can just make them immutable, if one likes to, but not the other way around.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 8, 2021

comment:7

Replying to @kliem:

I would prefer to not have another backend though.

I'm just saying that Polyhedron, if no backend is given and is_mutable=True is passed, should select one of the backends that support mutation. As of this or a follow-up ticket, I guess that's only ppl.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 13, 2021

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

8dceb88Polyhedron_interactive -> Polyhedron_mutable
5124844add _element_constructore_polyhedron for ppl over ZZ
c82c144remove copy paste typo
2cfd7a5Merge branch 'u/gh-kliem/small_improvements_ppl' of git://trac.sagemath.org/sage into u/gh-kliem/lazy_ppl
d4f6fb7remove redundant lines in test

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 13, 2021

Changed commit from 86be3ed to d4f6fb7

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 13, 2021

Changed commit from d4f6fb7 to 1ece615

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 13, 2021

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

1ece615allow ppl polyhedron to be mutable

@kliem

This comment has been minimized.

@kliem kliem changed the title Make ppl backend somewhat lazy Make ppl mutable Jul 13, 2021
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 13, 2021

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

7548c12invalidate dependent object on mutable polyhedron

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 13, 2021

Changed commit from 1ece615 to 7548c12

@kliem
Copy link
Contributor Author

kliem commented Jul 13, 2021

comment:13

I fixed faces to be unhashable when taken from a mutable polyhedron (or that actually happens automatically). This means that in particular we cannot create graphs or hasse diagrams for faces.

I also invalidate all objects that point towards the polyhedron, when the polyhedron is changed. I don't know if this is good or if a user is responsible for not keeping objects pointing on polyhedra that might change.

I also changed the behavior to not recycle Vrepresentation and Hrepresentation objects when clearing the cache. This why I at least prevent that an representation object that a user keeps changes at some point.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 15, 2021

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

8720572coverage in base.py
473f729remove unused imports
fce9f98check attribute first

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 15, 2021

Changed commit from 7548c12 to fce9f98

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 16, 2021

Reviewer: Matthias Koeppe

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 16, 2021

comment:15

LGTM.

@kliem
Copy link
Contributor Author

kliem commented Jul 16, 2021

comment:16

Thank you.

@vbraun
Copy link
Member

vbraun commented Jul 24, 2021

Changed branch from u/gh-kliem/lazy_ppl to fce9f98

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants