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

normaliz backend isn't ready for generators #29836

Closed
kliem opened this issue Jun 10, 2020 · 21 comments
Closed

normaliz backend isn't ready for generators #29836

kliem opened this issue Jun 10, 2020 · 21 comments

Comments

@kliem
Copy link
Contributor

kliem commented Jun 10, 2020

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).

CC: @jplab @LaisRast

Component: geometry

Keywords: polytopes, dilation

Author: Jonathan Kliem

Branch/Commit: 21f9d90

Reviewer: Matthias Koeppe

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

@kliem kliem added this to the sage-9.2 milestone Jun 10, 2020
@kliem
Copy link
Contributor Author

kliem commented Jun 10, 2020

comment:1

I just marked this as critical, because this is regression.

@kliem
Copy link
Contributor Author

kliem commented Jun 10, 2020

New commits:

cb13ddballow generators for Vrep/Hrep for backend normaliz

@kliem
Copy link
Contributor Author

kliem commented Jun 10, 2020

Commit: cb13ddb

@kliem
Copy link
Contributor Author

kliem commented Jun 10, 2020

Branch: public/29836

@fchapoton
Copy link
Contributor

comment:3

missing the # optional tag

@fchapoton
Copy link
Contributor

comment:4

and you could use

isinstance(thing, (A, B, C))

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 10, 2020

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

002e543added optional tag; small improvement

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 10, 2020

Changed commit from cb13ddb to 002e543

@kliem
Copy link
Contributor Author

kliem commented Jun 10, 2020

comment:6

Replying to @fchapoton:

and you could use

isinstance(thing, (A, B, C))

Much better. Thanks.

@mkoeppe
Copy link
Member

mkoeppe commented Jun 10, 2020

comment:7

Probably Polyhedron_base should get some _test_... methods to make sure that methods are tested with all backends instead of relying on coverage by copy-paste doctests.

@kliem
Copy link
Contributor Author

kliem commented Jun 10, 2020

comment:8

That would probably a good thing to do. I'm not familiar with TestSuite at all, but that should probably make things better.

@mkoeppe
Copy link
Member

mkoeppe commented Jun 10, 2020

comment:9

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.

@kliem
Copy link
Contributor Author

kliem commented Jun 10, 2020

comment:10

Thank you for the reference. I will do this, but better in a separate ticket.

@kliem
Copy link
Contributor Author

kliem commented Jun 10, 2020

comment:11

I opened #29842 for this.

@mkoeppe
Copy link
Member

mkoeppe commented Jun 10, 2020

Reviewer: Matthias Koeppe

@kliem
Copy link
Contributor Author

kliem commented Jun 10, 2020

comment:13

Thank you.

@kiwifb
Copy link
Member

kiwifb commented Jun 14, 2020

comment:14

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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 15, 2020

Changed commit from 002e543 to 21f9d90

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 15, 2020

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

21f9d90optional tag again

@kliem
Copy link
Contributor Author

kliem commented Jun 15, 2020

comment:16

Thanks for catching that.

@vbraun
Copy link
Member

vbraun commented Jun 23, 2020

Changed branch from public/29836 to 21f9d90

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

5 participants