Opened 2 years ago

Closed 9 months 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) Commit: 9f61dac34e2f4a755fbb8659bb2f7c6ab3130b5c
Dependencies: Stopgaps:

Description (last modified by mkoeppe)

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: Changed 2 years ago by jipilab

base_extend should do the job. But actually... the optional parameter to change the backend is simply not used at all!?!?!?!

sage: P = Polyhedron(vertices=[(1,0), (0,1)], rays=[(1,1)], base_ring=ZZ);  P
A 2-dimensional polyhedron in ZZ^2 defined as the convex hull of 2 vertices and 1 ray
sage: P_cdd = P.base_extend(base_ring=ZZ,backend='cdd')
sage: type(P_cdd)
<class 'sage.geometry.polyhedron.backend_ppl.Polyhedra_ZZ_ppl_with_category.element_class'>

This is because in the parent class, the method .base_extend() of parent does not even use the optional parameter backend...

sage: par = P.parent()
sage: par.base_extend??     # this is worth looking at...

I guess one could simply correct that...

Now the question is whether there is a smart way to go from one to another?

comment:2 Changed 2 years ago by mkoeppe

See also #22701.

comment:3 in reply to: ↑ 1 Changed 2 years ago by mforets

Replying to jipilab:

... This is because in the parent class, the method .base_extend() of parent does not even use the optional parameter backend...

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 16 months ago by jipilab

  • Description modified (diff)

comment:5 Changed 9 months ago by mkoeppe

  • 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 9 months ago by mkoeppe

  • Branch set to u/mkoeppe/fix_polyhedron_base_extend_so_it_does_not_ignore_the_backend_parameter

comment:7 Changed 9 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • 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: Changed 9 months ago by 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:

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 9 months ago by mkoeppe

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 9 months ago by git

  • Commit changed from 260739db6ac8ee46b50db4d1489d11b0621c1c36 to 9f61dac34e2f4a755fbb8659bb2f7c6ab3130b5c

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

9f61dacPolyhedron.base_extend docstring: Explain default value of backend

comment:11 Changed 9 months ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

Thanks. LGTM.

comment:12 Changed 9 months ago by vbraun

  • 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
Note: See TracTickets for help on using tickets.