Opened 10 months ago
Closed 10 months ago
#29836 closed defect (fixed)
normaliz backend isn't ready for generators
Reported by:  ghkliem  Owned by:  

Priority:  critical  Milestone:  sage9.2 
Component:  geometry  Keywords:  polytopes, dilation 
Cc:  jipilab, ghLaisRast  Merged in:  
Authors:  Jonathan Kliem  Reviewers:  Matthias Koeppe 
Report Upstream:  N/A  Work issues:  
Branch:  21f9d90 (Commits, GitHub, GitLab)  Commit:  21f9d90bba887bbd2d734c41273d0f6a2f2596a5 
Dependencies:  Stopgaps: 
Description
sage: P = polytopes.simplex(backend='normaliz') sage: K.<sqrt2> = QuadraticField(2) sage: P.dilation(sqrt2) Traceback (most recent call last): ... AttributeError: 'generator' object has no attribute 'list'
The reason for this is simple. With optimization in #29200, it seems that generators are passed down to the normaliz backend and this isn't ready for this yet (when converting the data to the normaliz field).
Change History (18)
comment:1 Changed 10 months ago by
comment:2 Changed 10 months ago by
 Branch set to public/29836
 Commit set to cb13ddb433735cd25e92a870385b9f7faa2875f1
 Status changed from new to needs_review
New commits:
cb13ddb  allow generators for Vrep/Hrep for backend normaliz

comment:3 Changed 10 months ago by
missing the # optional tag
comment:4 followup: ↓ 6 Changed 10 months ago by
and you could use
isinstance(thing, (A, B, C))
comment:5 Changed 10 months ago by
 Commit changed from cb13ddb433735cd25e92a870385b9f7faa2875f1 to 002e5436d235e9a6cd5dbde1b04356e33ac31afe
Branch pushed to git repo; I updated commit sha1. New commits:
002e543  added optional tag; small improvement

comment:6 in reply to: ↑ 4 Changed 10 months ago by
comment:7 Changed 10 months ago by
Probably Polyhedron_base
should get some _test_...
methods to make sure that methods are tested with all backends instead of relying on coverage by copypaste doctests.
comment:8 Changed 10 months ago by
That would probably a good thing to do. I'm not familiar with TestSuite? at all, but that should probably make things better.
comment:9 Changed 10 months ago by
Take a look at sage.numerical.backends.GenericBackend
. Some time ago I did a similar thing for the MIP backends there. There's also LoggingBackend
, which provides a semiautomatic way to make _test...
methods from existing doctests.
comment:10 Changed 10 months ago by
Thank you for the reference. I will do this, but better in a separate ticket.
comment:11 Changed 10 months ago by
I opened #29842 for this.
comment:12 Changed 10 months ago by
 Reviewers set to Matthias Koeppe
 Status changed from needs_review to positive_review
comment:13 Changed 10 months ago by
Thank you.
comment:14 Changed 10 months ago by
 Status changed from positive_review to needs_work
In
+ sage: P = polytopes.simplex(backend='normaliz') # optional  pynormaliz + sage: K.<sqrt2> = QuadraticField(2) # optional  pynormaliz + sage: P.dilation(sqrt2)
the last line needs to be optional as well. If you don't have pynormalize
you get a nice NameError: name P is not defined
message in your doctests.
comment:15 Changed 10 months ago by
 Commit changed from 002e5436d235e9a6cd5dbde1b04356e33ac31afe to 21f9d90bba887bbd2d734c41273d0f6a2f2596a5
Branch pushed to git repo; I updated commit sha1. New commits:
21f9d90  optional tag again

comment:16 Changed 10 months ago by
 Status changed from needs_work to needs_review
Thanks for catching that.
comment:17 Changed 10 months ago by
 Status changed from needs_review to positive_review
comment:18 Changed 10 months ago by
 Branch changed from public/29836 to 21f9d90bba887bbd2d734c41273d0f6a2f2596a5
 Resolution set to fixed
 Status changed from positive_review to closed
I just marked this as critical, because this is regression.