Opened 3 years ago
Closed 2 years ago
#25091 closed enhancement (fixed)
Expose some normaliz features
Reported by:  jipilab  Owned by:  

Priority:  major  Milestone:  sage8.8 
Component:  geometry  Keywords:  polytope, normaliz, IMAPolyGeom 
Cc:  vdelecroix, moritz, mkoeppe, ghsebasguts, Winfried, ghbraunmath, tscrim  Merged in:  
Authors:  JeanPhilippe Labbé, Matthias Koeppe  Reviewers:  Travis Scrimshaw, Jeroen Demeyer 
Report Upstream:  N/A  Work issues:  
Branch:  da05706 (Commits, GitHub, GitLab)  Commit:  da057067814b4a5adbc1e77ce0fc9dcc8f0dee83 
Dependencies:  #27682, #27716  Stopgaps: 
Description (last modified by )
Using Normaliz can compute several things that are currently not interfaced. This ticket implements:
 Integral points generators
 Euclidean volume and volume
 Triangulation
 Hilbert series
 Ehrhart series of compact polyhedron
 Ehrhart quasipolynomial of compact polyhedron
FOLLOWUP:
 Make ehrhart_polynomial uniform (in the spirit of
.volume()
that can have several engines.  Move the lattice related methods to
Polyhedra_normaliz_QQ
.
Change History (123)
comment:1 Changed 3 years ago by
 Branch set to u/jipilab/normaliz_features
 Commit set to e421ff89087f9d231056f163885b5ce251ed5d3b
 Dependencies set to #22984, #25090
comment:2 Changed 3 years ago by
 Commit changed from e421ff89087f9d231056f163885b5ce251ed5d3b to ac35212bdcb4b4671defd9e25ab90634d13f118d
comment:3 Changed 3 years ago by
 Branch changed from u/jipilab/normaliz_features to u/jipilab/some_normaliz_features
 Commit changed from ac35212bdcb4b4671defd9e25ab90634d13f118d to bf8b172f1a58a6d71b857575b00dd19b82f9d2e4
New commits:
bf8b172  Merge branch 'u/mkoeppe/pynormaliz2'

comment:4 Changed 3 years ago by
 Branch changed from u/jipilab/some_normaliz_features to public/some_normaliz_features
 Commit changed from bf8b172f1a58a6d71b857575b00dd19b82f9d2e4 to 9a6af25046563f288b4c93e61f0e3ab1fa593bf0
comment:5 Changed 3 years ago by
 Commit changed from 9a6af25046563f288b4c93e61f0e3ab1fa593bf0 to 3d3f64ab940e973a79557a37ddb7bb346b6b5898
Branch pushed to git repo; I updated commit sha1. New commits:
3d3f64a  Handle default behavior

comment:6 Changed 3 years ago by
 Commit changed from 3d3f64ab940e973a79557a37ddb7bb346b6b5898 to f376748a6c5330ea2f23d5e82de289ed7a575433
comment:7 followup: ↓ 21 Changed 3 years ago by
Note that is_package_installed
is being deprecated, see #20382
comment:8 Changed 3 years ago by
 Commit changed from f376748a6c5330ea2f23d5e82de289ed7a575433 to 55c9a04b9865076dc03dc3d701237ddaf5ee9614
Branch pushed to git repo; I updated commit sha1. New commits:
55c9a04  Added Hilbert series

comment:9 Changed 3 years ago by
 Commit changed from 55c9a04b9865076dc03dc3d701237ddaf5ee9614 to 720dc046b8831170f06fe341bdfa42fefeb240c4
Branch pushed to git repo; I updated commit sha1. New commits:
720dc04  Made tests pass

comment:10 Changed 3 years ago by
 Commit changed from 720dc046b8831170f06fe341bdfa42fefeb240c4 to a4ddeac32690b8be6a5edf66ff77979a1e9a2b3f
Branch pushed to git repo; I updated commit sha1. New commits:
a4ddeac  Cropped feature methods

comment:11 Changed 3 years ago by
 Commit changed from a4ddeac32690b8be6a5edf66ff77979a1e9a2b3f to 44549fa150a08fa327c7ae34f322bf4927b2eeec
comment:12 Changed 3 years ago by
 Commit changed from 44549fa150a08fa327c7ae34f322bf4927b2eeec to e32c9fddc39dd6780fea791796c5a536ecf4d42c
Branch pushed to git repo; I updated commit sha1. New commits:
e32c9fd  Added triangulate with normaliz

comment:13 Changed 3 years ago by
 Description modified (diff)
comment:14 Changed 3 years ago by
 Commit changed from e32c9fddc39dd6780fea791796c5a536ecf4d42c to 45c39b85af9485489e081b14af08fcde297c4bd5
Branch pushed to git repo; I updated commit sha1. New commits:
45c39b8  Some small edits

comment:15 Changed 3 years ago by
 Commit changed from 45c39b85af9485489e081b14af08fcde297c4bd5 to 56798216576de3521fc2276c734cbf6c70f14c62
Branch pushed to git repo; I updated commit sha1. New commits:
5679821  Added handling the triangulation with point configurations

comment:16 Changed 3 years ago by
 Commit changed from 56798216576de3521fc2276c734cbf6c70f14c62 to b045caf912268156ab2ea91b9efc425b8510df09
Branch pushed to git repo; I updated commit sha1. New commits:
b045caf  Adapted inhomogeneous case

comment:17 Changed 3 years ago by
 Description modified (diff)
comment:18 followup: ↓ 19 Changed 3 years ago by
It's probably best to add all latticepoint related methods to Polyhedron_QQ_normaliz
rather than Polyhedron_normaliz
in anticipation of #25097 (which moves integral_points
and integral_hull
there).
comment:19 in reply to: ↑ 18 ; followup: ↓ 20 Changed 3 years ago by
comment:20 in reply to: ↑ 19 Changed 3 years ago by
Replying to jipilab:
Replying to mkoeppe:
It's probably best to add all latticepoint related methods to
Polyhedron_QQ_normaliz
rather thanPolyhedron_normaliz
in anticipation of #25097 (which movesintegral_points
andintegral_hull
there).I see. Yes I can do that.
As discussed with mkoeppe: The move of the rational polyhedron methods will be done in #25097. The merge should go without conflicts.
comment:21 in reply to: ↑ 7 ; followup: ↓ 30 Changed 3 years ago by
 Dependencies changed from #22984, #25090 to #22984, #25090, #20382
 Description modified (diff)
Replying to vdelecroix:
Note that
is_package_installed
is being deprecated, see #20382
Oh! Wow! Thanks for the timely warning! I set it as a dependency. I will change the check once it is merged...
comment:22 Changed 3 years ago by
 Commit changed from b045caf912268156ab2ea91b9efc425b8510df09 to e7be14a7c0f2b3f2fc13cc2196b6f4cbad3bccdd
Branch pushed to git repo; I updated commit sha1. New commits:
e7be14a  Fixed some polytopes and added some tests

comment:23 Changed 3 years ago by
 Commit changed from e7be14a7c0f2b3f2fc13cc2196b6f4cbad3bccdd to 3615f8d33d9fb37ca0a905195dd21c8e17a05a7d
comment:24 Changed 3 years ago by
 Commit changed from 3615f8d33d9fb37ca0a905195dd21c8e17a05a7d to 8bff81f233fb570e8477db0ea1d898a8a611512d
comment:25 Changed 3 years ago by
 Commit changed from 8bff81f233fb570e8477db0ea1d898a8a611512d to d0e2498cf6a3e028c856702c23316e709fe17b00
Branch pushed to git repo; I updated commit sha1. New commits:
d0e2498  Added the Ehrhart Series and QuasiPolynomial

comment:26 Changed 3 years ago by
 Commit changed from d0e2498cf6a3e028c856702c23316e709fe17b00 to 668fdfc3d93a3c567c3258b7d8eaa2c36bcc5400
Branch pushed to git repo; I updated commit sha1. New commits:
668fdfc  Added an example

comment:27 Changed 3 years ago by
 Description modified (diff)
comment:28 Changed 3 years ago by
 Milestone changed from sage8.2 to sage8.4
comment:29 Changed 3 years ago by
Out of interest: Given a monomial ideal in Sage, how can one compute its Hilbert series using normaliz?
comment:30 in reply to: ↑ 21 ; followup: ↓ 37 Changed 3 years ago by
Replying to jipilab:
Replying to vdelecroix:
Note that
is_package_installed
is being deprecated, see #20382Oh! Wow! Thanks for the timely warning! I set it as a dependency. I will change the check once it is merged...
It is really a pity that #20382 doesn't explain (in the ticket description!!!) what one is supposed to do instead of is_package_installed
.
So, how would one change the current branch to make it use #20382?
comment:31 followup: ↓ 32 Changed 3 years ago by
Replying to SimonKing?
Normaliz can compute Hilbert series of the integral closures of monomial ideals, not of arbitrary monomial ideals. If one wants to apply it: input the monomials as vertices, the polynomial ring as cone (= unit matrix), and put the weights of the indeterminates into the grading.
Example for Normaliz input (integral closure of (x^{2, y}2, deg x = 2, deg y =1)
amb_space 2 vertices 2 2 0 1 0 2 1 cone 2 1 0 0 1 grading 2 1 HilbertSeries
comment:32 in reply to: ↑ 31 ; followup: ↓ 34 Changed 3 years ago by
Replying to Winfried:
Replying to SimonKing?
Normaliz can compute Hilbert series of the integral closures of monomial ideals, not of arbitrary monomial ideals. If one wants to apply it: input the monomials as vertices, the polynomial ring as cone (= unit matrix), and put the weights of the indeterminates into the grading.
Example for Normaliz input (integral closure of (x^{2, y}2, deg x = 2, deg y =1)
amb_space 2 vertices 2 2 0 1 0 2 1 cone 2 1 0 0 1 grading 2 1 HilbertSeries
Thank you. But my question was really: Given a monomial ideal in Sage, how to compute the Hilbert series of it (or maybe of its integral closure) using normaliz as a backend (not: in normaliz).
I.e.:
sage: n=4;m=11;P = PolynomialRing(QQ,n*m,"x"); x = P.gens(); M = Matrix(n,x) sage: I = P.ideal(M.minors(2)) sage: J = P*[m.lm() for m in I.groebner_basis()]
and what now? How to use the new interface to normaliz to compute the Hilbert series of (the integral closure of) J
?
Since I never worked with the integral closure of a monomial ideal: How are the Hilbert series of the ideal and its integral closure related?
comment:33 Changed 3 years ago by
"What now?" is a question that JeanPhilippe should answer. I have no overview which of the Normaliz computation goals can be reached from Sage. However, all of them can be reached via PyNormaliz?. For example, Normaliz can take a binomial ideal (suitably encoded) as input. It finds the affine monoid A defined by the binomial ideal, and then computes invariants of the normalization of A.
I think there is no way to relate the Hilbert series of an ideal I and the Hilbert series of its integral closure in a useful way.
comment:34 in reply to: ↑ 32 Changed 3 years ago by
Thank you. But my question was really: Given a monomial ideal in Sage, how to compute the Hilbert series of it (or maybe of its integral closure) using normaliz as a backend (not: in normaliz).
I.e.:
sage: n=4;m=11;P = PolynomialRing(QQ,n*m,"x"); x = P.gens(); M = Matrix(n,x) sage: I = P.ideal(M.minors(2)) sage: J = P*[m.lm() for m in I.groebner_basis()]and what now? How to use the new interface to normaliz to compute the Hilbert series of (the integral closure of)
J
?Since I never worked with the integral closure of a monomial ideal: How are the Hilbert series of the ideal and its integral closure related?
My knowledge of the theory lacks in order to give you a satisfactory answer to your questions.
The present ticket allows you to get the Hilbert series of a rational polyhedron. Whether this Hilbert series is or corresponds to the Hilbert series of a monomial ideal should be clarified. In case they match, then, what I would do in Sage is
1) a function that transfers the data of the monomial ideal to a polyhedron that uses the backend normaliz and compute its hilbert series (a function implemented in this ticket).
2) use PyNormaliz? package directly as the normaliz backend of polyhedron is currently doing. This is not really well documented as its usage is currently only in the backend for polyhedron.
For this, the present ticket provides some useful functions to deal with PyNormaliz? that builds on #25090.
comment:35 Changed 3 years ago by
Would this compute what you want? I did this with the current ticket and merge with the latest sage.
sage: n=4;m=11;P = PolynomialRing(QQ,n*m,"x"); x = P.gens(); M = Matrix(n,x) sage: I = P.ideal(M.minors(2)) sage: J = P*[m.lm() for m in I.groebner_basis()] sage: P = Polyhedron(rays=[m.degrees() for m in J.gens()],backend='normaliz') sage: P A 42dimensional polyhedron in ZZ^44 defined as the convex hull of 1 vertex and 330 rays sage: P.hilbert_series()
Well, I'd say this is a quite large computation involving triangulating a 42 dimensional polyhedron with 330 vertices...
comment:36 Changed 3 years ago by
What one really wants is not the Hilbert series of the ideal. The goal is the Hilbert series of the residue class ring defined by the ideal. The residue class ring can be identified with a monomial algebra of Krull dimension 14. The Hilbert series is easily calculated. If one wants to use Normaliz for it, then the best approach is to input the binomial ideal to Normaliz.
You make a matrix of integer vectors whose rows represent the binomials in the form vw, corresponding to x^{v  x}w. The input type is "lattice_ideal". You must also define a grading. Simply say "total_degree". In this case it represents a vector with 44 entries 1.
That one can go this way is due to the fact that the residue class ring is a normal monoid algebra. In general this is not the case.
comment:37 in reply to: ↑ 30 Changed 3 years ago by
Replying to SimonKing:
Replying to jipilab:
Replying to vdelecroix:
Note that
is_package_installed
is being deprecated, see #20382Oh! Wow! Thanks for the timely warning! I set it as a dependency. I will change the check once it is merged...
It is really a pity that #20382 doesn't explain (in the ticket description!!!) what one is supposed to do instead of
is_package_installed
.
Indeed.
comment:38 followup: ↓ 39 Changed 3 years ago by
Concerning the is_package_installed
thing, it seems that it is not deprecated when we want to check whether a package was installed using sage i
which is what we want here I guess.
So I would remove it from the necessary things to do for this ticket.
comment:39 in reply to: ↑ 38 ; followup: ↓ 40 Changed 3 years ago by
Replying to jipilab:
Concerning the
is_package_installed
thing, it seems that it is not deprecated when we want to check whether a package was installed usingsage i
which is what we want here I guess.So I would remove it from the necessary things to do for this ticket.
I tend to disagree. is_package_installed
is used here to test whether pynormaliz, normaliz, or latte_int is available. And it may be available for reasons that have nothing to do with sage i
.
comment:40 in reply to: ↑ 39 ; followups: ↓ 41 ↓ 42 Changed 3 years ago by
Replying to SimonKing:
Replying to jipilab:
Concerning the
is_package_installed
thing, it seems that it is not deprecated when we want to check whether a package was installed usingsage i
which is what we want here I guess.So I would remove it from the necessary things to do for this ticket.
I tend to disagree.
is_package_installed
is used here to test whether pynormaliz, normaliz, or latte_int is available. And it may be available for reasons that have nothing to do withsage i
.
Okay. Nevertheless, the mentionned ticket does not provide a way to check if they are available or not. Right?
comment:41 in reply to: ↑ 40 Changed 3 years ago by
Replying to jipilab:
Okay. Nevertheless, the mentionned ticket does not provide a way to check if they are available or not. Right?
Yes, it doesn't. But I guess that's because the author of that ticket thought that the solution is relatively straight forward: When you code in Python, how do you use a package? You do so by importing stuff. Hence, you should test whether the package is available by trying to import something from that package (sooner or later, you will do anyway!), surrounded with a try: ... except ImportError: ...
clause.
Indeed, in your code, you do import PyNormaliz
. So, this could be
try: import PyNormaliz except ImportError: raise ImportError("some meaningful error message, telling the user at least one way to install PyNormaliz")
For latte_int, I am not so sure what you do, because I don't see a related import statement in your code. However, in some place of your code, you do use it; you should try and see what happens if latte_int is not available: Will there be an ImportError, or a TypeError, or a ValueError, etc.? Then, instead of is_package_available
, you could try to use latte_int in a dry run, and if an error occurs, you take it as an answer (namely the answer that latte_int is not available).
comment:42 in reply to: ↑ 40 ; followup: ↓ 91 Changed 3 years ago by
Replying to jipilab:
Replying to SimonKing:
Replying to jipilab:
Concerning the
is_package_installed
thing, it seems that it is not deprecated when we want to check whether a package was installed usingsage i
which is what we want here I guess.So I would remove it from the necessary things to do for this ticket.
I tend to disagree.
is_package_installed
is used here to test whether pynormaliz, normaliz, or latte_int is available. And it may be available for reasons that have nothing to do withsage i
.Okay. Nevertheless, the mentionned ticket does not provide a way to check if they are available or not. Right?
I think what may be intended here is to subclass sage.features.PythonModule
.
comment:43 Changed 3 years ago by
 Commit changed from 668fdfc3d93a3c567c3258b7d8eaa2c36bcc5400 to d5fdf168e49a1a4260dc804d29998f33da3a4aba
Branch pushed to git repo; I updated commit sha1. New commits:
d5fdf16  Merge branch 'develop' into HEAD

comment:44 Changed 3 years ago by
 Commit changed from d5fdf168e49a1a4260dc804d29998f33da3a4aba to 20d2fde78750f859770f05473f49d7de460d6bb1
Branch pushed to git repo; I updated commit sha1. New commits:
20d2fde  Fix typo in docstring.

comment:45 Changed 3 years ago by
 Cc ghbraunmath added
comment:46 Changed 2 years ago by
I looked back at this now, and I'm getting a weird result from an example in Normaliz' manual:
sage: P = Polyhedron(vertices=[(1/2,1/2),(1/3,1/3),(1/4,1/2)],backend='normaliz') sage: P A 2dimensional polyhedron in QQ^2 defined as the convex hull of 3 vertices sage: P.hilbert_series() 0
From calling PyNormaliz?, I get:
sage: h = PyNormaliz.NmzResult(new_cone, "HilbertSeries") sage: h [[], [], 1L]
So it's not the parsing from PyNormaliz? to sage, but rather something weird is happening here. It has been a while since I wrote the code so I am a bit lost in trying to figure out what is going on...
I'll try to do some more searching...
comment:47 followup: ↓ 48 Changed 2 years ago by
I guess you want the Ehrhart series. The Normaliz computation goal HilbertSeries? requires the existence of a grading which is not defined with this input.
This is an intricate point, but it is inevitable: the grading is defined on the space in which the polyhedron lives, not on the cone over it.
comment:48 in reply to: ↑ 47 Changed 2 years ago by
Replying to Winfried:
I guess you want the Ehrhart series. The Normaliz computation goal HilbertSeries? requires the existence of a grading which is not defined with this input.
Yes, in the end I want the Ehrhart series, but I was doing both computations to understand their difference again...
This is an intricate point, but it is inevitable: the grading is defined on the space in which the polyhedron lives, not on the cone over it.
Yes, agreed. I take this example from p.23 of the Normaliz manual. I thought that HilbertSeries? would work out of the box like in the example. That's why I am asking myself what I did wrong.
In the code for hilbert_series, if no grading is given, we canonically give the all 1's grading on the ambient space, and then ask for the HilbertSeries?, in which case we get the empty result(!?). (Is that a good choice of canonical grading?)
So, my question would be, how do I get the result from the Normaliz manual?
This issue makes me wonder also if the results in the current examples are actually true (because maybe I am doing something wrong). For example, what would be the expected outputs of for the 3dimensional 01 cube?
Currently it gives me:
sage: C = Polyhedron(vertices = [[0,0,0],[0,0,1],[0,1,0],[0,1,1],[1,0,0],[1,0,1] ....: ,[1,1,0],[1,1,1]],backend='normaliz') sage: C.hilbert_series() t^3 + 3*t^2 + 3*t + 1 sage: C.ehrhart_series() (t^2 + 4*t + 1)/(t^10  t^9  t^8 + 2*t^5  t^2  t + 1) sage: C.ehrhart_series().denominator().factor() (t + 1)^2 * (t  1)^4 * (t^2 + 1) * (t^2 + t + 1) sage: C.ehrhart_quasipolynomial() t^3 + 3*t^2 + 3*t + 1
Hilbert series and ehrhart polynomials look fine, but the denominator of the Ehrhart series is not what I would expect. Is that related to the discussion on p.24 of Normaliz' manual?
I would have expected (1t)^{4, so that the degree would be 2 so that when we dilate by a factor two, we see the first interior integer point. Right? }
comment:49 Changed 2 years ago by
HilbertSeries? works out of the box, provided you use the right input type:
polytope defines a cone and a grading, and the Hilbert series of the cone under this grading is the Ehrhart series of the polytope that arises as the intersection of the cone and the hyperplane of degree 1 elements. polytope is a homogeneous input type.
The inhomogeneous type vertices only defines a polytope P, or in connection with cone a potentially umbounded polyhedron. We can have an grding on the space in which the polytope lives, and then HilbertSeries? makes sense. This Hilbert series is the generating function of the lattice point enumerator of the points in P. You can ask Normaliz to compute the Ehrhart series of P if P is bounded. It constructs a cone with a grading behind the scenes and computes the Hilbert series of that cone.
So far, so good.
In your cube example: the vertices are in principle rational vectors. What are your denominators? Does Sage choose them automatically? If so what are they?
Moreover, it seems to me tat Sage implicitly defines a grading on the space of the polytpe, taking the sum of the coordinates. This explains why you get a Hilbert series and that it is exactly the result shown: 1 point of degree 0, 3 points of degree 1, 3 of degree 2 and 1 of degree 3.
The denominator of the Ehrhart series is for a 3dimensional polytope that has vertices of degree 4,3,2 qnd 1.
Before I can say more, I must know what happens on the way from Sage input to Normaliz input.
comment:50 Changed 2 years ago by
PS. My question for the denominators must be understood form the viewpoint of Normaliz input. For the 3cube we need vertices with four coordinates in Normaliz where the last coordinate represents a denominator for the first three. (This was introduced before Normaliz allowed fractions in input; it would be superfluous now.)
If you send the matrix with the 8 vertices as it is to Normaliz as input type vertices, then the result will be a BadInputexception?, because we have the denominator 0 for some vertices. In other words: Something must have happened from the Sage vertices before they go into Normaliz.
I think you want the denominator 1 attached to each vertex. If we do that, the Normaliz input becomes
amb_space auto vertices [[0,0,0,1],[0,0,1,1],[0,1,0,1],[0,1,1,1],[1,0,0,1],[1,0,1,1],[1,1,0,1],[1,1,1,1]] EhrhartSeries
and the output contains
Ehrhart series: 1 4 1 denominator with 4 factors: 1: 4
as one expects.
Again my question: what happens on the way from Sage input to Normaliz input?
comment:51 Changed 2 years ago by
Dear Winfried,
Thanks for your quick reply. This explanation is really good, and probably I should add it to the top to shortly describe how the translation from sage to normaliz is done.
Here is the verbose of the example for the cube, we can see the input given by sage to normaliz. The current way is to always use inhomogeneous (vertices, cones, and subspaces). Eventually, inhom. equations and inequalities are added.
sage: C = Polyhedron(vertices = [[0,0,0],[0,0,1],[0,1,0],[0,1,1],[1,0,0],[1,0,1],[1,1,0],[1,1,1]],backend='normaliz', ....: verbose=True) # Calling PyNormaliz.NmzCone(**{'subspace': [], 'vertices': [[0, 0, 0, 1], [0, 0, 1, 1], [0, 1, 0, 1], [0, 1, 1, 1], [1, 0, 0, 1], [1, 0, 1, 1], [1, 1, 0, 1], [1, 1, 1, 1]], 'cone': []}) # 8< Equivalent Normaliz input file 8< amb_space 3 subspace 0 vertices 8 0 0 0 1 0 0 1 1 0 1 0 1 0 1 1 1 1 0 0 1 1 0 1 1 1 1 0 1 1 1 1 1 cone 0 # 8<8<8<
Once I have this, I compute the hilbert series, without giving any grading. In this case, Sage assign the grading [1,...,1] and passes it to normaliz (as you observed) and get:
sage: C.hilbert_series() t^3 + 3*t^2 + 3*t + 1
Now, for the Ehrhart series, I get:
sage: C.ehrhart_series() (t^2 + 4*t + 1)/(t^10  t^9  t^8 + 2*t^5  t^2  t + 1)
As you said:
It constructs a cone with a grading behind the scenes and computes the Hilbert series of that cone.
What is the grading in this case?
So there seems to be a difference between the above input and the one you mentioned:
amb_space auto vertices [[0,0,0,1],[0,0,1,1],[0,1,0,1],[0,1,1,1],[1,0,0,1],[1,0,1,1],[1,1,0,1],[1,1,1,1]] EhrhartSeries
because they give different output for the Ehrhart series?
comment:52 followup: ↓ 53 Changed 2 years ago by
The Normaliz input is the desired one. If I run it with the computation goal EhhhartSeries? I get the right answer, and if I run it with the computation goal HilbertSeries?, then the answer is that it cannot be computed. Everything o.k.
But it is also clear that this workflow is not exactly that of Sage/PyNormaliz?: the latter constructs a cone and then calls functions that trigger computations and return results. For a better analysis the next step is to put the same data into PyNormaliz? and to see what happens.
comment:53 in reply to: ↑ 52 Changed 2 years ago by
Replying to Winfried:
The Normaliz input is the desired one. If I run it with the computation goal EhhhartSeries? I get the right answer, and if I run it with the computation goal HilbertSeries?, then the answer is that it cannot be computed. Everything o.k.
Ok. Good to know!
But it is also clear that this workflow is not exactly that of Sage/PyNormaliz?: the latter constructs a cone and then calls functions that trigger computations and return results. For a better analysis the next step is to put the same data into PyNormaliz? and to see what happens.
Ok, I will look into that.
comment:54 Changed 2 years ago by
Ok, I think that I have figured out something, that I did not expect.
sage: C = Polyhedron(vertices = [[0,0,0],[0,0,1],[0,1,0],[0,1,1],[1,0,0],[1,0,1],[1,1,0],[1,1,1] ....: ],backend='normaliz',verbose=True) # Calling PyNormaliz.NmzCone(**{'subspace': [], 'vertices': [[0, 0, 0, 1], [0, 0, 1, 1], [0, 1, 0, 1], [0, 1, 1, 1], [1, 0, 0, 1], [1, 0, 1, 1], [1, 1, 0, 1], [1, 1, 1, 1]], 'cone': []}) # 8< Equivalent Normaliz input file 8< amb_space 3 subspace 0 vertices 8 0 0 0 1 0 0 1 1 0 1 0 1 0 1 1 1 1 0 0 1 1 0 1 1 1 1 0 1 1 1 1 1 cone 0 # 8<8<8<
So far I simply created the 01cube using normaliz. Then, I get the normaliz cone and the enclosed data:
sage: cone = C._normaliz_cone;cone <capsule object "Cone" at 0x7fcc880cf210> sage: data = C._get_nmzcone_data()
From there, if I compute the Hilbert Series directly, it will complain, because it does not have a grading:
sage: PyNormaliz.NmzResult(cone,"HilbertSeries")  error Traceback (most recent call last) <ipythoninput2693c9989a5377> in <module>() > 1 PyNormaliz.NmzResult(cone,"HilbertSeries") error: Could not compute: No grading specified and cannot find one. Cannot compute some requested properties!
So far so good. Then, I ask for the Ehrhart series and then, it unlocks the Hilbert series:
sage: PyNormaliz.NmzResult(cone,"EhrhartSeries") [[1L, 4L, 1L], [1L, 1L, 1L, 1L], 0L] sage: PyNormaliz.NmzResult(cone,"HilbertSeries") [[1L, 4L, 1L], [1L, 1L, 1L, 1L], 0L]
For now, there is no way to access the difference in the normaliz cone. It is now equipped with a grading in the background, but it seems that we can not extract it:
sage: new_C = C.parent().element_class._from_normaliz_cone(parent=C.parent(),normaliz_cone=cone) sage: PyNormaliz.NmzResult(new_C._normaliz_cone,"HilbertSeries") [[1L, 4L, 1L], [1L, 1L, 1L, 1L], 0L] sage: PyNormaliz.NmzResult(new_C._normaliz_cone,"Grading")  error Traceback (most recent call last) <ipythoninput43809cb91899e5> in <module>() > 1 PyNormaliz.NmzResult(new_C._normaliz_cone,"Grading") error: Could not compute: Grading !
But, when creating a whole new cone from the data and adding by hand the grading (which is what is done in hilbert_series
in sage, we get the enumeration of the lattice points with respect to this grading:
sage: C.hilbert_series() t^3 + 3*t^2 + 3*t + 1
This can be reproduced manually as:
sage: data['grading'] = [1] * C.ambient_dim() sage: new_cone = C._make_normaliz_cone(data) sage: h = PyNormaliz.NmzResult(new_cone, "HilbertSeries");h [[1L, 3L, 3L, 1L], [], 0L]
So, right now, we get two potential output for Hilbert series.
Further, if we actually change the normaliz cone while doing the computation of the Hilbert series (instead of doing it on a distinct cone created on the side), it then breaks the ehrhart series:
sage: C._normaliz_cone = new_cone sage: C.ehrhart_series()  error Traceback (most recent call last) <ipythoninput850117d93a819f> in <module>() > 1 C.ehrhart_series() /home/jplabbe/sage/local/lib/python2.7/sitepackages/sage/geometry/polyhedron/backend_normaliz.pyc in ehrhart_series(self, variable) 636 637 cone = self._normaliz_cone > 638 e = PyNormaliz.NmzResult(cone, "EhrhartSeries") 639 640 from sage.rings.polynomial.polynomial_ring_constructor import PolynomialRing error: Some error in the normaliz input data detected: Grading not allowed with Ehrhart series in the inhomogeneous case
Oi Oi Oi...
comment:55 Changed 2 years ago by
Everything above is as it should be, with 2 exceptions:
1) The computation of the Ehrhart series should not make the Hilbert series computed. This requires a correction in libnormaliz.
There is no grading in the background. The Ehrhart series was computed on an auxiliary cone.
2) Sage should not add a default grading if one asks for the Hilbert series. This is not in my hands.
Further remark: if one changes the grading later on (or defines one), all grading dependent results are erased.
comment:56 Changed 2 years ago by
PS. The last observation is the intended behavior of Normaliz: HilbertSeries? and EhrhartSeries? are not allowed on the same input. One intended obstruction to this is that we do not even allow a grading if EhrhartSeries? is to be computed.
comment:57 Changed 2 years ago by
The next Normaliz release will store the Hilbert series and the Ehrhart series separately and provide a specific access function for the Ehrhart series. Then the confusion that we see in the preceding remarks will not arise anymore.
comment:58 Changed 2 years ago by
Ok!!
So I went back to PyNormaliz? and had a look at the tutorial mentioned in Appendix E of the normaliz manual:
https://mybinder.org/v2/gh/Normaliz/NormalizJupyter/master
In there, the PyNormaliz? output of .hilbert_series is explained. Importantly, it seems that the output is different than the one of normaliz(!). This was the source of one of the above problems.
[[1L, 4L, 1L], [1L, 1L, 1L, 1L], 0L]
As to be interpreted as t^2+4t+1
divided by (1t)(1t)(1t)(1t)
and a 0
shift.
So, it's all good. But it indicates that the code should be more explicit on those output formats... I'll put this on my list of TODO to add on some comments on the pynormaliz side...
comment:59 followup: ↓ 61 Changed 2 years ago by
In the Normaliz pitput file we use a multiset notation. i think this is difficult to represent in Python. Therefore it is translated into a vector format where each element of the multiset is repeated as often as it occurs in the multiset.
comment:60 Changed 2 years ago by
 Commit changed from 20d2fde78750f859770f05473f49d7de460d6bb1 to f36bfeb89fd9293a021a0bb2f8f9584ba1a5ca10
comment:61 in reply to: ↑ 59 Changed 2 years ago by
Replying to Winfried:
In the Normaliz pitput file we use a multiset notation. i think this is difficult to represent in Python. Therefore it is translated into a vector format where each element of the multiset is repeated as often as it occurs in the multiset.
Yes, makes sense now. Somehow got it wrong from the beginning. I just commited an update that should cover the mistake. I added several examples coming from normaliz manual to be able to compare.
I still have to smoothen the usage for the user. But now, at least things look like they work and make sense...
comment:62 Changed 2 years ago by
 Description modified (diff)
comment:63 Changed 2 years ago by
 Commit changed from f36bfeb89fd9293a021a0bb2f8f9584ba1a5ca10 to dbf7db846182a3bc496458d8eea1bf10e8792b0a
Branch pushed to git repo; I updated commit sha1. New commits:
dbf7db8  Fixed the Feature of Latte

comment:64 Changed 2 years ago by
 Description modified (diff)
comment:65 Changed 2 years ago by
Here is a bug, which I think is coming from the PyNormaliz? side of things:
sage: v = [[0,0,0],[0,0,1],[0,1,0],[0,1,1],[1,0,0],[1,0,1],[1,1,0],[1,1,1]] sage: cube = Polyhedron(vertices=v,backend='normaliz') sage: cube._volume_normaliz() sage: cube._volume_normaliz('induced_lattice') 6 sage: cube._volume_normaliz() 1.0
Equivalently:
sage: v = [[0,0,0],[0,0,1],[0,1,0],[0,1,1],[1,0,0],[1,0,1],[1,1,0],[1,1,1]] sage: cube = Polyhedron(vertices=v,backend='normaliz',verbose=True) # Calling PyNormaliz.NmzCone(**{'subspace': [], 'vertices': [[0, 0, 0, 1], [0, 0, 1, 1], [0, 1, 0, 1], [0, 1, 1, 1], [1, 0, 0, 1], [1, 0, 1, 1], [1, 1, 0, 1], [1, 1, 1, 1]], 'cone': []}) # 8< Equivalent Normaliz input file 8< amb_space 3 subspace 0 vertices 8 0 0 0 1 0 0 1 1 0 1 0 1 0 1 1 1 1 0 0 1 1 0 1 1 1 1 0 1 1 1 1 1 cone 0 # 8<8<8< sage: cone = cube._normaliz_cone sage: import PyNormaliz sage: PyNormaliz.NmzResult(cone,'EuclideanVolume') sage: PyNormaliz.NmzResult(cone,'EuclideanVolume') 1.0
This is weird. The volume somehow seems to get lost on the way. When one asks again, the output appears.
comment:66 Changed 2 years ago by
 Commit changed from dbf7db846182a3bc496458d8eea1bf10e8792b0a to 25b17381036c5e0bd792437b3dda0efde017fa4c
Branch pushed to git repo; I updated commit sha1. New commits:
25b1738  Fixed some doc

comment:67 Changed 2 years ago by
 Commit changed from 25b17381036c5e0bd792437b3dda0efde017fa4c to 21671bdf4561edcbf07617f526d649e962038dc2
comment:68 Changed 2 years ago by
 Milestone changed from sage8.4 to sage8.7
comment:69 Changed 2 years ago by
This
try: import PyNormaliz except ImportError: raise ImportError("This backend requires PyNormaliz. To install PyNormaliz, type 'sage i pynormaliz' in the terminal.")
is not desirable. You should do that via a "feature". See the class PythonModule
in sage/features/__init__.py
. The main reason is that sage i
would only work for people compiling from source (which is the case of less and less people nowadays).
comment:70 Changed 2 years ago by
 Milestone changed from sage8.7 to sage8.8
Ticket retargeted after milestone closed (if you don't believe this ticket is appropriate for the Sage 8.8 release please retarget manually)
comment:71 Changed 2 years ago by
 Report Upstream changed from N/A to Fixed upstream, in a later stable release.
 Work issues set to Some doctest fails, will need to be based on a newer version
comment:72 Changed 2 years ago by
 Dependencies changed from #22984, #25090, #20382 to #22984, #25090, #20382, #27682
comment:73 Changed 2 years ago by
I'd suggest to merge #27716 as a dependency and use the new helper method _nmz_result
comment:74 Changed 2 years ago by
 Commit changed from 21671bdf4561edcbf07617f526d649e962038dc2 to 86ce4420c5f2f9b4d72f8afe7c01764eea5391b0
comment:75 Changed 2 years ago by
 Dependencies changed from #22984, #25090, #20382, #27682 to #22984, #25090, #20382, #27682, #27716
comment:76 Changed 2 years ago by
 Commit changed from 86ce4420c5f2f9b4d72f8afe7c01764eea5391b0 to 79059033a2b14ba9ffaf032a45361e12817efc75
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
0dfec03  _format_function_call: Fix doctest

b370268  pyflakes fixes

328a04a  _nmz_result: Add doctest

ee5ff41  Mark newest doctest # optional  pynormaliz

78c2de4  Merge branch 't/27716/public/normalizbackendrefactoring' into t/25091/public/some_normaliz_features

41bd968  Fix doctest error message  NormalizError

7662077  Fixup merge

885ef9e  Use EhrhartQuasiPolynomial

313fee1  Fix doctest of hilbert_series

7905903  Pass grading as a matrix

comment:77 Changed 2 years ago by
 Commit changed from 79059033a2b14ba9ffaf032a45361e12817efc75 to 9249dda436e7e9c58b644ea5e4728f8fe969949f
Branch pushed to git repo; I updated commit sha1. New commits:
9249dda  Use _nmz_result instead of calling NmzResult directly

comment:78 Changed 2 years ago by
 Dependencies changed from #22984, #25090, #20382, #27682, #27716 to #27682, #27716
 Report Upstream changed from Fixed upstream, in a later stable release. to N/A
 Status changed from new to needs_review
 Work issues Some doctest fails, will need to be based on a newer version deleted
comment:79 Changed 2 years ago by
 Cc tscrim added
comment:80 Changed 2 years ago by
I have some comments:
This is a general Python convention we try to follow with the messages, moreover, the error is because the class has the wrong type (you are not expecting it to be implemented at some point, right?):
raise NotImplementedError("The backend should be normaliz.") +raise TypeError("the backend should be normaliz")
A similar comment applies elsewhere too. However, the ImportError
can be the exception to this guide as it is "long".
The INPUT:
bullet points are not considered to be complete sentences, so they should not end in a period/fullstop.
PEP8:
sage: S = Polyhedron(vertices = [[0,1],[1,0]],backend='normaliz') # optional  pynormaliz +sage: S = Polyhedron(vertices=[[0,1],[1,0]], backend='normaliz') # optional  pynormaliz
and similar such things.
In ehrhart_series()
, i think this will look better in latex formatting:
``t^k`` is number of integer lattice points inside the ``k``th dilation of +`t^k` is number of integer lattice points inside the `k`th dilation of
Note that k
and t
are not input variable names.
comment:81 Changed 2 years ago by
 Commit changed from 9249dda436e7e9c58b644ea5e4728f8fe969949f to adc3f2a4e6d82427850517738d1681300565a675
comment:82 Changed 2 years ago by
 Commit changed from adc3f2a4e6d82427850517738d1681300565a675 to 970b46dafef7d96fdcb9d0b97edb8c6b132cc473
Branch pushed to git repo; I updated commit sha1. New commits:
970b46d  Fixed coverage and blocks

comment:83 Changed 2 years ago by
 Status changed from needs_review to needs_work
There's an error in a doctest.
sage t long src/sage/geometry/polyhedron/base.py ********************************************************************** File "src/sage/geometry/polyhedron/base.py", line 2874, in sage.geometry.polyhedron.base.Polyhedron_base._triangulate_normaliz Failed example: K._triangulate_normaliz(engine='normaliz') Expected: Traceback (most recent call last): ... TypeError: The polyhedron's backend should be 'normaliz' Got: <BLANKLINE> Traceback (most recent call last): File "/Users/mkoeppe/s/sage/sagerebasing/worktreealgebraic2018spring/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 671, in _run self.compile_and_execute(example, compiler, test.globs) File "/Users/mkoeppe/s/sage/sagerebasing/worktreealgebraic2018spring/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 1095, in compile_and_execute exec(compiled, globs) File "<doctest sage.geometry.polyhedron.base.Polyhedron_base._triangulate_normaliz[1]>", line 1, in <module> K._triangulate_normaliz(engine='normaliz') TypeError: _triangulate_normaliz() got an unexpected keyword argument 'engine' **********************************************************************
comment:84 Changed 2 years ago by
 Commit changed from 970b46dafef7d96fdcb9d0b97edb8c6b132cc473 to e1eee578228315cab288ef5a0d22aca81a473731
Branch pushed to git repo; I updated commit sha1. New commits:
e1eee57  Fixed some doctests

comment:85 Changed 2 years ago by
 Status changed from needs_work to needs_review
comment:86 followup: ↓ 88 Changed 2 years ago by
I think I ended up being a little vague about the error messages. In Python, error messages start with a lowercase letter and do not end with a period/fullstop. While I do not expect you to change them all, I think it is good not to make it further from the general convention. I know this is not documented, but is sort of an agreement. So if you want to keep things the way they are, then you can go ahead and set a positive review. Although I would appreciate you changing them (or at least making a uniform convention for all of your added error messages).
comment:87 Changed 2 years ago by
One comment:
+# FLINT is a standard package and EANTIC is a standard pakcage. We
the 2nd occ. of standard > optional
comment:88 in reply to: ↑ 86 Changed 2 years ago by
Replying to tscrim:
I think I ended up being a little vague about the error messages. In Python, error messages start with a lowercase letter and do not end with a period/fullstop. While I do not expect you to change them all, I think it is good not to make it further from the general convention. I know this is not documented, but is sort of an agreement. So if you want to keep things the way they are, then you can go ahead and set a positive review. Although I would appreciate you changing them (or at least making a uniform convention for all of your added error messages).
Oh, I see. No problem, I will change it.
comment:89 Changed 2 years ago by
 Status changed from needs_review to needs_work
This is still using is_package_installed
.
comment:90 Changed 2 years ago by
Instead of putting Latte().require()
in a try
block, you can just check if Latte().is_present()
It can be argued which kind of error should be given here (I would go for RuntimeError
), but certainly not a TypeError
:
raise TypeError("The induced rational measure can only be computed with the optional packages `latte_int`, or `pynormaliz`")
comment:91 in reply to: ↑ 42 Changed 2 years ago by
Replying to mkoeppe:
I think what may be intended here is to subclass
sage.features.PythonModule
.
Not subclassing, just use it like PythonModule("package.submodule").is_present()
or whatever.
comment:92 Changed 2 years ago by
More in detail: replace
try: import PyNormaliz except ImportError: raise ImportError("This backend requires PyNormaliz. To install PyNormaliz, type 'sage i pynormaliz' in the terminal.")
by
from sage.features import PythonModule PythonModule("PyNormaliz", spkg="pynormaliz").require() import PyNormaliz
comment:93 Changed 2 years ago by
 Commit changed from e1eee578228315cab288ef5a0d22aca81a473731 to a4f01d474d6c5a18ce88ce6bd5a2c65dff7bac29
comment:94 Changed 2 years ago by
 Status changed from needs_work to needs_review
I fixed the error messages, typos, and the require()
of PyNormaliz
.
Ready for review again.
comment:96 Changed 2 years ago by
 Commit changed from a4f01d474d6c5a18ce88ce6bd5a2c65dff7bac29 to fbd3dce364043bbd59dc10dbd1a742fec0d937a6
Branch pushed to git repo; I updated commit sha1. New commits:
fbd3dce  removed is_package_installed

comment:97 Changed 2 years ago by
 Status changed from needs_work to needs_review
... forgot about this latte/pynormaliz switch there. Hopefully complete now.
comment:98 Changed 2 years ago by
I would still recommend to change the type of exception: TypeError
makes no sense here.
comment:99 Changed 2 years ago by
 Commit changed from fbd3dce364043bbd59dc10dbd1a742fec0d937a6 to 8ceee8473f0b43e9b738923d1de353e66ed84f0a
Branch pushed to git repo; I updated commit sha1. New commits:
8ceee84  Change TypeError for RuntimeError

comment:100 Changed 2 years ago by
... sorry for the omission, and thanks for the coaching on the features module.
comment:101 Changed 2 years ago by
Thank you for changing the error message strings.
You also need to make a changes in the combinat/rigged_configurations/kleber_tree.py
try: poly = Polyhedron(ieqs=ieqs, backend='normaliz') self._has_normaliz = True  except ImportError: + except FeatureNotPresentError: poly = Polyhedron(ieqs=ieqs) self._has_normaliz = False
to reflect the new error message is thrown. That should take care of all of the errors reported on the patchbot (for when normaliz
is not installed).
Modulo that, I am happy with this and am willing to set a positive review. Jeroen, any other comments?
comment:102 Changed 2 years ago by
 Commit changed from 8ceee8473f0b43e9b738923d1de353e66ed84f0a to 0da067f203fce15a1f27f480e1f2dd29bc8bd669
Branch pushed to git repo; I updated commit sha1. New commits:
0da067f  Fixed doctests and added author name

comment:103 Changed 2 years ago by
 Commit changed from 0da067f203fce15a1f27f480e1f2dd29bc8bd669 to d7122d75e8149e4324e31928a313623ad427817b
Branch pushed to git repo; I updated commit sha1. New commits:
d7122d7  pyflakes

comment:104 Changed 2 years ago by
You forgot to import FeatureNotPresentError
in kleber_tree.py
.
comment:105 Changed 2 years ago by
 Commit changed from d7122d75e8149e4324e31928a313623ad427817b to 63a80b6634df8ee26bb0a50056956aec2c1a4b3a
Branch pushed to git repo; I updated commit sha1. New commits:
63a80b6  Forgot an import

comment:106 Changed 2 years ago by
... it has been a long workshop.
comment:107 Changed 2 years ago by
 Commit changed from 63a80b6634df8ee26bb0a50056956aec2c1a4b3a to d8716e3b8d51da404a63f7ca747847088a8d0d2a
Branch pushed to git repo; I updated commit sha1. New commits:
d8716e3  Refactor NmzResult

comment:108 Changed 2 years ago by
The last commit refactors things in the spirit of #27716.
It is now completely ready for review... No more forgotten things hopefully.
comment:109 followups: ↓ 110 ↓ 112 Changed 2 years ago by
 Reviewers set to Travis Scrimshaw, Jeroen Demeyer
 Status changed from needs_review to needs_work
You definitely seem to have done quite a bit in your workshop. However, now there are serious things failing: I am getting an infinite recursion happening that ends with
/home/uqtscrim/sagebuild/local/lib/python2.7/sitepackages/sage/geometry/polyhedron/backend_normaliz.pyc in _nmz_result(self, normaliz_cone, property) 193 PythonModule("PyNormaliz", spkg="pynormaliz").require() 194 import PyNormaliz > 195 return self._nmz_result(normaliz_cone, property) 196 197 def _init_from_normaliz_cone(self, normaliz_cone): ... last 1 frames repeated, from the frame below ... /home/uqtscrim/sagebuild/local/lib/python2.7/sitepackages/sage/geometry/polyhedron/backend_normaliz.pyc in _nmz_result(self, normaliz_cone, property) 193 PythonModule("PyNormaliz", spkg="pynormaliz").require() 194 import PyNormaliz > 195 return self._nmz_result(normaliz_cone, property) 196 197 def _init_from_normaliz_cone(self, normaliz_cone): RuntimeError: maximum recursion depth exceeded in cmp
A function that only calls itself will definitely infinitely recurse. ;)
Additionally, a couple of little details noted by the patchbot from pyflakes for backend_normaliz.py
:
In def _cone_generators(pynormaliz_cone):
, you have
return self._nmz_result(pynormaliz_cone, "Generators")
In particular, self
is not been defined in this @staticmethod
.
Do you require the import PyNormaliz
here in _nmz_result
:
PythonModule("PyNormaliz", spkg="pynormaliz").require() import PyNormaliz return self._nmz_result(normaliz_cone, property)
comment:110 in reply to: ↑ 109 Changed 2 years ago by
Oï oï oï... Will fix this, asap.
Replying to tscrim:
You definitely seem to have done quite a bit in your workshop. However, now there are serious things failing: I am getting an infinite recursion happening that ends with
/home/uqtscrim/sagebuild/local/lib/python2.7/sitepackages/sage/geometry/polyhedron/backend_normaliz.pyc in _nmz_result(self, normaliz_cone, property) 193 PythonModule("PyNormaliz", spkg="pynormaliz").require() 194 import PyNormaliz > 195 return self._nmz_result(normaliz_cone, property) 196 197 def _init_from_normaliz_cone(self, normaliz_cone): ... last 1 frames repeated, from the frame below ... /home/uqtscrim/sagebuild/local/lib/python2.7/sitepackages/sage/geometry/polyhedron/backend_normaliz.pyc in _nmz_result(self, normaliz_cone, property) 193 PythonModule("PyNormaliz", spkg="pynormaliz").require() 194 import PyNormaliz > 195 return self._nmz_result(normaliz_cone, property) 196 197 def _init_from_normaliz_cone(self, normaliz_cone): RuntimeError: maximum recursion depth exceeded in cmpA function that only calls itself will definitely infinitely recurse.
;)
Additionally, a couple of little details noted by the patchbot from pyflakes for
backend_normaliz.py
:In
def _cone_generators(pynormaliz_cone):
, you havereturn self._nmz_result(pynormaliz_cone, "Generators")In particular,
self
is not been defined in this@staticmethod
.Do you require the
import PyNormaliz
here in_nmz_result
:PythonModule("PyNormaliz", spkg="pynormaliz").require() import PyNormaliz return self._nmz_result(normaliz_cone, property)
comment:111 Changed 2 years ago by
 Commit changed from d8716e3b8d51da404a63f7ca747847088a8d0d2a to 08b315bd50d9ae5737f99a2d92d6abe2708a38f3
Branch pushed to git repo; I updated commit sha1. New commits:
08b315b  Fixed infinite recursion

comment:112 in reply to: ↑ 109 Changed 2 years ago by
Replying to tscrim:
A function that only calls itself will definitely infinitely recurse.
;)
I love those...
Additionally, a couple of little details noted by the patchbot from pyflakes for
backend_normaliz.py
:In
def _cone_generators(pynormaliz_cone):
, you havereturn self._nmz_result(pynormaliz_cone, "Generators")In particular,
self
is not been defined in this@staticmethod
.
Is now fixed.
Do you require the
import PyNormaliz
here in_nmz_result
:PythonModule("PyNormaliz", spkg="pynormaliz").require() import PyNormaliz return self._nmz_result(normaliz_cone, property)
This was the cause of the infinite recursion. It is now fixed.
comment:113 Changed 2 years ago by
 Status changed from needs_work to needs_review
comment:114 Changed 2 years ago by
Needs review again!
comment:115 followup: ↓ 116 Changed 2 years ago by
 Status changed from needs_review to positive_review
LGTM.
comment:116 in reply to: ↑ 115 Changed 2 years ago by
comment:117 followup: ↓ 118 Changed 2 years ago by
 Status changed from positive_review to needs_work
********************************************************************** File "src/sage/geometry/polyhedron/base.py", line 5295, in sage.geometry.polyhedron.base.Polyhedron_base.volume Failed example: P.volume(measure='induced_rational') # optional  latte_int Expected: 3/2 Got: 1 ********************************************************************** 1 item had failures: 1 of 47 in sage.geometry.polyhedron.base.Polyhedron_base.volume [1137 tests, 1 failure, 48.32 s]  sage t long src/sage/geometry/polyhedron/base.py # 1 doctest failed 
comment:118 in reply to: ↑ 117 ; followup: ↓ 119 Changed 2 years ago by
Replying to vbraun:
********************************************************************** File "src/sage/geometry/polyhedron/base.py", line 5295, in sage.geometry.polyhedron.base.Polyhedron_base.volume Failed example: P.volume(measure='induced_rational') # optional  latte_int Expected: 3/2 Got: 1 ********************************************************************** 1 item had failures: 1 of 47 in sage.geometry.polyhedron.base.Polyhedron_base.volume [1137 tests, 1 failure, 48.32 s]  sage t long src/sage/geometry/polyhedron/base.py # 1 doctest failed 
Hm. Annoying.
I can reproduce this with optional=4ti2,bliss,dochtml,e_antic,gfortran,latte_int,lidia,lrslib,memlimit,mpir,ninja_build,python2,sage,topcom
and all tests pass with pynormaliz
added. Dammit.
comment:119 in reply to: ↑ 118 Changed 2 years ago by
Replying to jipilab:
Replying to vbraun:
********************************************************************** File "src/sage/geometry/polyhedron/base.py", line 5295, in sage.geometry.polyhedron.base.Polyhedron_base.volume Failed example: P.volume(measure='induced_rational') # optional  latte_int Expected: 3/2 Got: 1 ********************************************************************** 1 item had failures: 1 of 47 in sage.geometry.polyhedron.base.Polyhedron_base.volume [1137 tests, 1 failure, 48.32 s]  sage t long src/sage/geometry/polyhedron/base.py # 1 doctest failed Hm. Annoying.
I can reproduce this with
optional=4ti2,bliss,dochtml,e_antic,gfortran,latte_int,lidia,lrslib,memlimit,mpir,ninja_build,python2,sage,topcom
and all tests pass with
pynormaliz
added. Dammit.
Got it! It's from a previous test... That's a easy fix. I forgot to put both latte_it and pynormaliz as optional...
comment:120 Changed 2 years ago by
 Commit changed from 08b315bd50d9ae5737f99a2d92d6abe2708a38f3 to da057067814b4a5adbc1e77ce0fc9dcc8f0dee83
Branch pushed to git repo; I updated commit sha1. New commits:
da05706  Fix forgotten optional package test

comment:121 Changed 2 years ago by
 Status changed from needs_work to needs_review
Needs review again...
comment:122 Changed 2 years ago by
 Status changed from needs_review to positive_review
comment:123 Changed 2 years ago by
 Branch changed from public/some_normaliz_features to da057067814b4a5adbc1e77ce0fc9dcc8f0dee83
 Resolution set to fixed
 Status changed from positive_review to closed
Last 10 new commits:
Updating patch with upstream fix for wrong number of lattice points.
Merge branch 'u/tscrim/upgrade_noramliz_pynormaliz22984' of git://trac.sagemath.org/sage into u/tscrim/upgrade_noramliz_pynormaliz22984
Upgrade Normaliz to 3.5.2.
Adding tests from comment:24,25 of #22984.
Upgrade PyNormaliz to 1.12
Adapted the polyhedron docstring
Upgrade normaliz to 3.5.3
Merge branch to get docstring adaptation
Merge branch 'develop' into test_normaliz
First version of integral pts gen