Opened 6 years ago
Closed 6 years ago
#22522 closed enhancement (fixed)
generic latte_int interface to integrate
Reported by:  Marcelo Forets  Owned by:  

Priority:  major  Milestone:  sage7.6 
Component:  packages: optional  Keywords:  days84 
Cc:  Vincent Delecroix, Matthias Köppe, JeanPhilippe Labbé  Merged in:  
Authors:  Marcelo Forets  Reviewers:  Matthias Koeppe 
Report Upstream:  N/A  Work issues:  
Branch:  6a474f9 (Commits, GitHub, GitLab)  Commit:  6a474f92580914f1cf42595fd8ee7b882b244b5d 
Dependencies:  #22497  Stopgaps: 
Description (last modified by )
Provide a generic interface to integrate a polynomial over a polytope.
See also #18232.
Change History (34)
comment:1 Changed 6 years ago by
Dependencies:  → #22497 

comment:2 Changed 6 years ago by
Description:  modified (diff) 

Keywords:  days84 added 
comment:3 Changed 6 years ago by
Branch:  → u/mforets/22497 

comment:4 Changed 6 years ago by
Commit:  → 96e40991054c847b0d1fb97623aa36a61854411a 

Status:  new → needs_review 
comment:5 Changed 6 years ago by
Commit:  96e40991054c847b0d1fb97623aa36a61854411a → d3c95899bcff12e6a17dccd849a35d9274bb7bbd 

Branch pushed to git repo; I updated commit sha1. New commits:
d3c9589  Add volume function to generic latte_int interface.

comment:6 followup: 10 Changed 6 years ago by
Why your commit d3c9589
is here!? Shouldn't it belong to another ticket?
comment:7 Changed 6 years ago by
Commit:  d3c95899bcff12e6a17dccd849a35d9274bb7bbd → 000bf8b56094cb83b11894e2c8195747e06c803f 

Branch pushed to git repo; I updated commit sha1. New commits:
000bf8b  Restructured the script, with an integrate function accepting different valuations.

comment:8 Changed 6 years ago by
Commit:  000bf8b56094cb83b11894e2c8195747e06c803f → 5aa669584a92f474fc1a797a3caad9c03c92d6cc 

Branch pushed to git repo; I updated commit sha1. New commits:
5aa6695  Added test for helper function _to_latte_polynomial.

comment:9 Changed 6 years ago by
Commit:  5aa669584a92f474fc1a797a3caad9c03c92d6cc → 72d03a12c6fc8b02e67a97d61538ca47c3996c3e 

Branch pushed to git repo; I updated commit sha1. New commits:
72d03a1  Minor changes to helper function and integrate.

comment:10 Changed 6 years ago by
Replying to vdelecroix:
Why your commit
d3c9589
is here!? Shouldn't it belong to another ticket?
The branch is on top of the branch of #22497.
comment:11 Changed 6 years ago by
Reviewers:  → Matthias Koeppe 

Status:  needs_review → positive_review 
Looks good to me.
Now followup tickets with corresponding methods of Polyhedron
would be good.
comment:12 Changed 6 years ago by
Commit:  72d03a12c6fc8b02e67a97d61538ca47c3996c3e → 6a474f92580914f1cf42595fd8ee7b882b244b5d 

Status:  positive_review → needs_review 
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
6a474f9  fixed a typo in the docstring

comment:14 Changed 6 years ago by
Commit:  6a474f92580914f1cf42595fd8ee7b882b244b5d → 17911f7051d618046f8ed37681b6f4516ec751b3 

Branch pushed to git repo; I updated commit sha1. New commits:
17911f7  Added integrate method to Polyhedron base.py

comment:15 Changed 6 years ago by
Replying to mkoeppe:
What does
``[1,[2,2,2]]``do in the doctest?
It's me struggling to get tests work in the particular case where a string is passed as argument :(
Probably some issue with string conversion. See this:
In the Sage REPL, the usual single quotes work, as in:
sage: P = polytopes.cuboctahedron() sage: P.integrate('[[3,[2,4,6]],[7,[0, 3, 5]]]') 629/47775
(ok I'm already using #20887 for P.integrate(..) but you get the point).
However the doctests didn't pass if I used that, but they pass with the double quote in one test. In Python it seems to have a meaning:
sage: type(``x``) <type 'str'>
But I'd like to understand what went wrong with just single quotes, of course.
comment:16 Changed 6 years ago by
Learn about :: in the doc formatting. This will solve your problem. Also build and verify the html doc.
comment:17 Changed 6 years ago by
There are also a number of doc issues with integrate
in base.py
. The doc should start with
def integrate(self, polynomial, **kwds): r""" Return the integral of a polynomial over a polytope. INPUT:  ``polynomial``  a multivariate polynomial or a valid LattE description string for a polynomial  ``**kwds``  additional keyword arguments that are passed to the engine OUTPUT: The integral of the polynomial over the polytope. .. NOTE:: The polytope triangulation algorithm is used. This function depends on LattE (i.e., the ``latte_int`` optional package).
The code in the EXAMPLES::
block also needs to be indented.
comment:18 followup: 21 Changed 6 years ago by
Commit:  17911f7051d618046f8ed37681b6f4516ec751b3 → 507aea5adb6f05ddc95cfef7cd79d24d3041de10 

comment:19 followup: 20 Changed 6 years ago by
done! and thanks both for the help. from my side this ticket and #20887 are ok.
in the proposed solution, the new volume and integrate methods can also receive polyhedra in RDF, and each vertex is cast to QQ with an iterator over the vertices.
comment:20 followup: 23 Changed 6 years ago by
Replying to mforets:
done! and thanks both for the help. from my side this ticket and #20887 are ok.
Nice work!
in the proposed solution, the new volume and integrate methods can also receive polyhedra in RDF, and each vertex is cast to QQ with an iterator over the vertices.
Is the answer of .integrate()
will be a RDF? Otherwise I think it is a bad idea.
comment:21 Changed 6 years ago by
Replying to git:
Branch pushed to git repo; I updated commit sha1. New commits:
91d885e Fix the docstrings, and enhance integrate method.
507aea5 Add the new volume engine Polyhedron.
I don't understand why these commits are here... this ticket is about generic latte_int interface to integrate
. If you change the purpose of the ticket you need to update the description.
comment:22 Changed 6 years ago by
Status:  needs_review → needs_work 

comment:23 followup: 24 Changed 6 years ago by
Replying to vdelecroix:
Replying to mforets:
done! and thanks both for the help. from my side this ticket and #20887 are ok.
Nice work!
in the proposed solution, the new volume and integrate methods can also receive polyhedra in RDF, and each vertex is cast to QQ with an iterator over the vertices.
Is the answer of
.integrate()
will be a RDF? Otherwise I think it is a bad idea.
But why? It will be a QQ.
comment:24 Changed 6 years ago by
Replying to mforets:
Replying to vdelecroix:
Replying to mforets:
done! and thanks both for the help. from my side this ticket and #20887 are ok.
Nice work!
in the proposed solution, the new volume and integrate methods can also receive polyhedra in RDF, and each vertex is cast to QQ with an iterator over the vertices.
Is the answer of
.integrate()
will be a RDF? Otherwise I think it is a bad idea.But why? It will be a QQ.
It does not make any sense. If I have a Polyhedron defined over RDF (= non exact field) its volume can not defined as an element of QQ (= exact field).
comment:25 Changed 6 years ago by
Hmm.. Matrices have a change_ring feature that comes very handy for conversion. Shall we add a similar one for Polyhedron?
And following your recommendation, make integrate and volume via latte
raise a value error when instantiated with, say, P = 1.4142*polytopes.cube()
.
With the change ring, we could still integrate integrate a polynomial over P with the slightly more complicated P.change_ring().integrate(x^2*y^2*z^2)
, instead of the ugly
sage: P_QQ = Polyhedron(vertices = [[QQ(vi) for vi in v] for v in P.vertices_generator()]) sage: P_QQ.integrate(x^2*y^2*z^2)
If there are alternative recommended options, I'd really like to learn; but I can post it as a ask.sagemath one if i'm offtopic :)
comment:26 Changed 6 years ago by
Cc:  JeanPhilippe Labbé added 

Integration over an RDF polyhedron via QQ polyhedron (latte) could make sense as long as the method converts the answer back to RDF to indicate that the answer is inexact.
As to change_ring
, this could be useful. I had related discussions with JP.
comment:28 Changed 6 years ago by
Commit:  507aea5adb6f05ddc95cfef7cd79d24d3041de10 → a563192b0dcbce9ab4e1387a1f3435e5d179cbc9 

Branch pushed to git repo; I updated commit sha1. New commits:
a563192  fix a bug in _volume_latte

comment:29 Changed 6 years ago by
Branch:  u/mforets/22497 → u/mforets/22522 

Commit:  a563192b0dcbce9ab4e1387a1f3435e5d179cbc9 → 6a474f92580914f1cf42595fd8ee7b882b244b5d 
comment:30 Changed 6 years ago by
Status:  needs_work → needs_review 

comment:31 Changed 6 years ago by
Status:  needs_review → needs_work 

comment:32 Changed 6 years ago by
Status:  needs_work → needs_review 

comment:33 Changed 6 years ago by
Status:  needs_review → positive_review 

comment:34 Changed 6 years ago by
Branch:  u/mforets/22522 → 6a474f92580914f1cf42595fd8ee7b882b244b5d 

Resolution:  → fixed 
Status:  positive_review → closed 
New commits:
22497: generic interface to LattE integrale count
Integral of a polynomial over a polytope.