Opened 5 years ago
Closed 4 years ago
#22575 closed enhancement (fixed)
Fix Polyhedron.base_extend so it does not ignore the backend parameter
Reported by: | mforets | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.4 |
Component: | geometry | Keywords: | days84, polytope |
Cc: | mkoeppe, jipilab, tscrim | Merged in: | |
Authors: | Matthias Koeppe | Reviewers: | Travis Scrimshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | 9f61dac (Commits, GitHub, GitLab) | Commit: | 9f61dac34e2f4a755fbb8659bb2f7c6ab3130b5c |
Dependencies: | Stopgaps: |
Description (last modified by )
Polyhedra can be instantiated with different backends (CDD, PPL, normaliz, ...), and the base_extend
method is supposed to allow to transform between backends, but the parameter is simply ignored (in (Polyhedra_base.base_extend
).
Possible follow-up: Also a new method change_backend
could be implemented as an alias. (Once done, add it to tutorial, too.)
Change History (12)
comment:1 follow-up: ↓ 3 Changed 5 years ago by
comment:2 Changed 5 years ago by
See also #22701.
comment:3 in reply to: ↑ 1 Changed 5 years ago by
Replying to jipilab:
... This is because in the parent class, the method
.base_extend()
ofparent
does not even use the optional parameterbackend
...sage: par = P.parent() sage: par.base_extend?? # this is worth looking at...
good finding, maybe this was a feature that didn't get in at the time.
also, i didn't find a command to get the backend as a string, like P.backend()
, or P.get_backend()
. it seems that there is no object (list) of available_backends
either.
I guess one could simply correct that...
Now the question is whether there is a smart way to go from one to another?
i suggest to just start with the naive way and see how it behaves.
comment:4 Changed 4 years ago by
- Description modified (diff)
comment:5 Changed 4 years ago by
- Cc tscrim added
- Description modified (diff)
- Milestone changed from sage-7.6 to sage-8.4
- Summary changed from Add .change_backend() method for polyhedra to Fix Polyhedron.base_extend so it does not ignore the backend parameter
comment:6 Changed 4 years ago by
- Branch set to u/mkoeppe/fix_polyhedron_base_extend_so_it_does_not_ignore_the_backend_parameter
comment:7 Changed 4 years ago by
- Commit set to 260739db6ac8ee46b50db4d1489d11b0621c1c36
- Description modified (diff)
- Status changed from new to needs_review
New commits:
260739d | {Polyhedron_base, Polyhedron_base}.base_extend: Handle backend.
|
comment:8 follow-up: ↓ 9 Changed 4 years ago by
So my only quip is that this defaults to the automatic backend choice when the backend is not specified. I would expect that the backend be the same backend as what I am currently working with:
sage: P = Polyhedron(vertices=[(1,0), (0,1)], rays=[(1,1)], base_ring=QQ) sage: P.backend() 'ppl' sage: P = Polyhedron(vertices=[(1,0), (0,1)], rays=[(1,1)], base_ring=ZZ, backend='normaliz') sage: P.backend() 'normaliz' sage: P.base_extend(QQ).backend() 'ppl'
So if you think this is the behavior that it should have, then you should put a .. WARNING::
explaining that the backend is given by the default if not specified.
comment:9 in reply to: ↑ 8 Changed 4 years ago by
Replying to tscrim:
So my only quip is that this defaults to the automatic backend choice when the backend is not specified. I would expect that the backend be the same backend as what I am currently working with:
One might expect that, but the backend may simply not be suitable. Consider base_extend of a PPL polytope to AA - only the 'field' backend is suitable.
I'll add some documentation.
comment:10 Changed 4 years ago by
- Commit changed from 260739db6ac8ee46b50db4d1489d11b0621c1c36 to 9f61dac34e2f4a755fbb8659bb2f7c6ab3130b5c
Branch pushed to git repo; I updated commit sha1. New commits:
9f61dac | Polyhedron.base_extend docstring: Explain default value of backend
|
comment:11 Changed 4 years ago by
- Reviewers set to Travis Scrimshaw
- Status changed from needs_review to positive_review
Thanks. LGTM.
comment:12 Changed 4 years ago by
- Branch changed from u/mkoeppe/fix_polyhedron_base_extend_so_it_does_not_ignore_the_backend_parameter to 9f61dac34e2f4a755fbb8659bb2f7c6ab3130b5c
- Resolution set to fixed
- Status changed from positive_review to closed
base_extend
should do the job. But actually... the optional parameter to change the backend is simply not used at all!?!?!?!This is because in the parent class, the method
.base_extend()
ofparent
does not even use the optional parameterbackend
...I guess one could simply correct that...
Now the question is whether there is a smart way to go from one to another?