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

py3: polyhedron pynormaliz backend does not handle large coefficients correctly #28370

Closed
videlec opened this issue Aug 20, 2019 · 10 comments
Closed

Comments

@videlec
Copy link
Contributor

videlec commented Aug 20, 2019

sage: x = polygen(ZZ)
sage: K.<sqrt2> = NumberField(x^2 - 2, embedding=AA(2).sqrt())
sage: P = Polyhedron(vertices=[(2**100, sqrt2), (sqrt2,1)], backend='normaliz')
Error in rational_handler: Python int too large to convert to C long
Error in nfelem_handler: unsupported operand type(s) for +=: 'NoneType' and 'sage.rings.number_field.number_field_element_quadratic.NumberFieldElement_quadratic'
Error in rational_handler: Python int too large to convert to C unsigned long
Error in rational_handler: Python int too large to convert to C unsigned long
Error in nfelem_handler: unsupported operand type(s) for *: 'NoneType' and 'sage.rings.number_field.number_field_element_quadratic.NumberFieldElement_quadratic'
Error in rational_handler: Python int too large to convert to C long
Error in rational_handler: Python int too large to convert to C long
Error in nfelem_handler: unsupported operand type(s) for *: 'NoneType' and 'sage.rings.number_field.number_field_element_quadratic.NumberFieldElement_quadratic'
Error in rational_handler: Python int too large to convert to C long
Error in rational_handler: Python int too large to convert to C long
Error in nfelem_handler: unsupported operand type(s) for *: 'NoneType' and 'sage.rings.number_field.number_field_element_quadratic.NumberFieldElement_quadratic'
sage: P.vertices()
(A vertex at (sqrt2, 1), A vertex at (0, sqrt2))

Even though PyNormaliz is perfectly fine working with large coefficients

sage: cone = PyNormaliz.NmzCone(cone=[],
....:          number_field=['a^2 - 2', 'a', '[1.414213562373095 +/- 2.99e-16]'],
....:          vertices=[[1267650600228229401496703205376r, [[0r, 1r], [1r, 1r]], 1r], [[[0r, 1r], [1r, 1r]], 1r, 1r]])
sage: PyNormaliz.NmzResult(cone, "VerticesOfPolyhedron")
[[[[0, 1], [1, 1]], [[1, 1], [0, 1]], [[1, 1], [0, 1]]],
 [[[1267650600228229401496703205376, 1], [0, 1]],
  [[0, 1], [1, 1]],
  [[1, 1], [0, 1]]]]
sage: PyNormaliz.NmzResult(cone, "SupportHyperplanes")
[[[[0, 1267650600228229401496703205376],
   [-1, 1267650600228229401496703205376]],
  [[1, 1], [0, 1]],
  [[0, 1], [0, 1]]],
 [[[0, 2], [1, 2]], [[-1, 1], [0, 1]], [[0, 1], [0, 1]]]]

Depends on #28321

CC: @mkoeppe @jplab

Component: geometry

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

@videlec videlec added this to the sage-8.9 milestone Aug 20, 2019
@videlec

This comment has been minimized.

@videlec
Copy link
Contributor Author

videlec commented Aug 20, 2019

Dependencies: #28321

@videlec
Copy link
Contributor Author

videlec commented Aug 20, 2019

comment:2

I just think that #28321 is to blame (Python 3 issue)

@videlec
Copy link
Contributor Author

videlec commented Aug 20, 2019

Author: Vincent Delecroix

@videlec videlec changed the title Polyhedron pynormaliz backend does not handle large coefficients correctly py3: polyhedron pynormaliz backend does not handle large coefficients correctly Aug 20, 2019
@jplab
Copy link

jplab commented Aug 20, 2019

comment:3

You mean that it would be a duplicate? i.e. that the backend is fine?

@videlec
Copy link
Contributor Author

videlec commented Aug 20, 2019

comment:4

It is probably not a problem with the backend.

@videlec
Copy link
Contributor Author

videlec commented Aug 20, 2019

comment:5

I confirm that it is solved with #28321... however, the fact that errors are not raised in the constructor is a bug in itself. In the example I wrote in the ticket description, I obtain a polytope P which is simply wrong. It is to my mind much better to get an error rather than false results.

@videlec
Copy link
Contributor Author

videlec commented Aug 20, 2019

Changed author from Vincent Delecroix to none

@videlec videlec removed this from the sage-8.9 milestone Aug 20, 2019
@jplab
Copy link

jplab commented Aug 20, 2019

comment:7

Replying to @videlec:

I confirm that it is solved with #28321... however, the fact that errors are not raised in the constructor is a bug in itself. In the example I wrote in the ticket description, I obtain a polytope P which is simply wrong. It is to my mind much better to get an error rather than false results.

I agree. Where to raise this error? This should become another ticket?

@videlec
Copy link
Contributor Author

videlec commented Aug 20, 2019

comment:8

Replying to @jplab:

Replying to @videlec:

I confirm that it is solved with #28321... however, the fact that errors are not raised in the constructor is a bug in itself. In the example I wrote in the ticket description, I obtain a polytope P which is simply wrong. It is to my mind much better to get an error rather than false results.

I agree. Where to raise this error? This should become another ticket?

  • Another ticket: yes
  • What to do: just remove the try/except

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

3 participants