Opened 3 years ago

Closed 3 years ago

#22522 closed enhancement (fixed)

generic latte_int interface to integrate

Reported by: mforets Owned by:
Priority: major Milestone: sage-7.6
Component: packages: optional Keywords: days84
Cc: vdelecroix, mkoeppe, jipilab Merged in:
Authors: Marcelo Forets Reviewers: Matthias Koeppe
Report Upstream: N/A Work issues:
Branch: 6a474f9 (Commits) Commit: 6a474f92580914f1cf42595fd8ee7b882b244b5d
Dependencies: #22497 Stopgaps:

Description (last modified by vdelecroix)

Provide a generic interface to integrate a polynomial over a polytope.

See also #18232.

Change History (34)

comment:1 Changed 3 years ago by mforets

  • Dependencies set to #22497

comment:2 Changed 3 years ago by vdelecroix

  • Description modified (diff)
  • Keywords days84 added

comment:3 Changed 3 years ago by mforets

  • Branch set to u/mforets/22497

comment:4 Changed 3 years ago by mforets

  • Commit set to 96e40991054c847b0d1fb97623aa36a61854411a
  • Status changed from new to needs_review

New commits:

d5ff15422497: generic interface to LattE integrale count
96e4099Integral of a polynomial over a polytope.

comment:5 Changed 3 years ago by git

  • Commit changed from 96e40991054c847b0d1fb97623aa36a61854411a to d3c95899bcff12e6a17dccd849a35d9274bb7bbd

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

d3c9589Add volume function to generic latte_int interface.

comment:6 follow-up: Changed 3 years ago by vdelecroix

Why your commit d3c9589 is here!? Shouldn't it belong to another ticket?

comment:7 Changed 3 years ago by git

  • Commit changed from d3c95899bcff12e6a17dccd849a35d9274bb7bbd to 000bf8b56094cb83b11894e2c8195747e06c803f

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

000bf8bRestructured the script, with an integrate function accepting different valuations.

comment:8 Changed 3 years ago by git

  • Commit changed from 000bf8b56094cb83b11894e2c8195747e06c803f to 5aa669584a92f474fc1a797a3caad9c03c92d6cc

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

5aa6695Added test for helper function _to_latte_polynomial.

comment:9 Changed 3 years ago by git

  • Commit changed from 5aa669584a92f474fc1a797a3caad9c03c92d6cc to 72d03a12c6fc8b02e67a97d61538ca47c3996c3e

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

72d03a1Minor changes to helper function and integrate.

comment:10 in reply to: ↑ 6 Changed 3 years ago by mkoeppe

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 3 years ago by mkoeppe

  • Reviewers set to Matthias Koeppe
  • Status changed from needs_review to positive_review

Looks good to me.

Now follow-up tickets with corresponding methods of Polyhedron would be good.

comment:12 Changed 3 years ago by git

  • Commit changed from 72d03a12c6fc8b02e67a97d61538ca47c3996c3e to 6a474f92580914f1cf42595fd8ee7b882b244b5d
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

6a474f9fixed a typo in the docstring

comment:13 follow-up: Changed 3 years ago by mkoeppe

What does

``[1,[2,2,2]]``

do in the doctest?

comment:14 Changed 3 years ago by git

  • Commit changed from 6a474f92580914f1cf42595fd8ee7b882b244b5d to 17911f7051d618046f8ed37681b6f4516ec751b3

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

17911f7Added integrate method to Polyhedron base.py

comment:15 in reply to: ↑ 13 Changed 3 years ago by mforets

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 3 years ago by mkoeppe

Learn about :: in the doc formatting. This will solve your problem. Also build and verify the html doc.

comment:17 Changed 3 years ago by tscrim

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 (as per comment:16).

Last edited 3 years ago by tscrim (previous) (diff)

comment:18 follow-up: Changed 3 years ago by git

  • Commit changed from 17911f7051d618046f8ed37681b6f4516ec751b3 to 507aea5adb6f05ddc95cfef7cd79d24d3041de10

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

91d885eFix the docstrings, and enhance integrate method.
507aea5Add the new volume engine Polyhedron.

comment:19 follow-up: Changed 3 years ago by mforets

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 in reply to: ↑ 19 ; follow-up: Changed 3 years ago by 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.

comment:21 in reply to: ↑ 18 Changed 3 years ago by vdelecroix

Replying to git:

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

91d885eFix the docstrings, and enhance integrate method.
507aea5Add 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 3 years ago by vdelecroix

  • Status changed from needs_review to needs_work

comment:23 in reply to: ↑ 20 ; follow-up: Changed 3 years ago by 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.

comment:24 in reply to: ↑ 23 Changed 3 years ago by vdelecroix

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 3 years ago by mforets

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 off-topic :)

comment:26 Changed 3 years ago by mkoeppe

  • Cc jipilab 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:27 Changed 3 years ago by mkoeppe

(In addition to change_ring, there could also be change_backend.)

comment:28 Changed 3 years ago by git

  • Commit changed from 507aea5adb6f05ddc95cfef7cd79d24d3041de10 to a563192b0dcbce9ab4e1387a1f3435e5d179cbc9

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

a563192fix a bug in _volume_latte

comment:29 Changed 3 years ago by mforets

  • Branch changed from u/mforets/22497 to u/mforets/22522
  • Commit changed from a563192b0dcbce9ab4e1387a1f3435e5d179cbc9 to 6a474f92580914f1cf42595fd8ee7b882b244b5d

comment:30 Changed 3 years ago by mforets

  • Status changed from needs_work to needs_review

comment:31 Changed 3 years ago by mforets

  • Status changed from needs_review to needs_work

comment:32 Changed 3 years ago by mforets

  • Status changed from needs_work to needs_review

comment:33 Changed 3 years ago by mkoeppe

  • Status changed from needs_review to positive_review

comment:34 Changed 3 years ago by vbraun

  • Branch changed from u/mforets/22522 to 6a474f92580914f1cf42595fd8ee7b882b244b5d
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.