Opened 4 years ago
Closed 2 years ago
#22574 closed enhancement (fixed)
Add .change_ring() method for polyhedra
Reported by:  mforets  Owned by:  

Priority:  major  Milestone:  sage8.7 
Component:  geometry  Keywords:  days84, polytope 
Cc:  mkoeppe, jipilab  Merged in:  
Authors:  Laith Rastanawi  Reviewers:  JeanPhilippe Labbé 
Report Upstream:  N/A  Work issues:  
Branch:  c9b92db (Commits, GitHub, GitLab)  Commit:  c9b92dbc55f09e73e00c307bf19fa59d52d898e0 
Dependencies:  Stopgaps: 
Description (last modified by )
Polyhedra can be defined in different rings, and this method allows to transform between rings (compare to the similar feature for matrices).
TODO:
 Once done, add it to tutorial
Change History (13)
comment:1 Changed 4 years ago by
comment:2 followup: ↓ 3 Changed 4 years ago by
I think there is no value in trying to implement inplace
 the various base rings are handled by different classes after all. Moreover, right now polyhedra are immutable  and it would be strange to be able to mutate the base ring but nothing else.
comment:3 in reply to: ↑ 2 Changed 4 years ago by
Replying to mkoeppe:
I think there is no value in trying to implement
inplace
 the various base rings are handled by different classes after all. Moreover, right now polyhedra are immutable  and it would be strange to be able to mutate the base ring but nothing else.
Agreed.
Further, I don't really like the fact that hidden in base_extend
is the possibility to change the backend, which as a matter of fact doesn't work. This is not userfriendly to say the least. I would therefore also deprecate the keyword completely and point to a .change_backend()
method (see #22575). I guess that such a method will take care to handle the modifications of base ring if need be...
comment:4 Changed 3 years ago by
 Description modified (diff)
comment:5 Changed 2 years ago by
 Branch set to u/ghLaisRast/change_ring
 Commit set to 5f617670bb3af64f3fb1519f562a5e4dd5666c4d
New commits:
5f61767  add change_ring to base.py and parent.py

comment:6 Changed 2 years ago by
 Commit changed from 5f617670bb3af64f3fb1519f562a5e4dd5666c4d to 6cdd9d551bbbb0521c1b08aa959ffeb51ce299f3
Branch pushed to git repo; I updated commit sha1. New commits:
6cdd9d5  add documentation

comment:7 Changed 2 years ago by
 Status changed from new to needs_review
comment:8 Changed 2 years ago by
 Status changed from needs_review to needs_work
Hi,
Here are a couple of things I saw:
 Adapt the convention for the input, see for example
vertex_facet_graph
:+  ``backend``  the new backend, see + :func:`~sage.geometry.polyhedron.constructor.Polyhedron`. + If ``None`` (the default), use the same defaulting behavior + as described there; it is not attempted to keep the same + backend.
 It would be good to have more examples with all the potential pairings.
 Further, it would be nice to handle the potential error in the function when coercing from
QQ
toZZ
and return a message like: 'Can not change the ring toZZ
: a coordinate is rational'. And these examples should be added after the examples that do work.
comment:9 Changed 2 years ago by
 Commit changed from 6cdd9d551bbbb0521c1b08aa959ffeb51ce299f3 to c9b92dbc55f09e73e00c307bf19fa59d52d898e0
Branch pushed to git repo; I updated commit sha1. New commits:
c9b92db  Add more debug output/Add more documentation

comment:10 Changed 2 years ago by
 Milestone changed from sage7.6 to sage8.7
 Status changed from needs_work to needs_review
comment:11 Changed 2 years ago by
 Reviewers set to JeanPhilippe Labbé
comment:12 Changed 2 years ago by
 Status changed from needs_review to positive_review
This looks good to me.
comment:13 Changed 2 years ago by
 Branch changed from u/ghLaisRast/change_ring to c9b92dbc55f09e73e00c307bf19fa59d52d898e0
 Resolution set to fixed
 Status changed from positive_review to closed
There is currently:
Could there be a boolean parameter such as
inplace
(similar as in graphs and simplicial complexes...) to determine if one should create a new object or just change the base ring?I am wondering how much changing the base ring changes the internals of the polyhedron object. Would many things break? We can try and see...