Opened 3 years ago

Closed 2 years ago

#29196 closed enhancement (fixed)

Make Ehrhart series related function cached

Reported by: jipilab Owned by:
Priority: major Milestone: sage-9.2
Component: geometry Keywords: ehrhart, h star vector
Cc: selia, gh-kliem Merged in:
Authors: Jean-Philippe Labbé, Jonathan Kliem Reviewers: Matthias Koeppe
Report Upstream: N/A Work issues:
Branch: b397c07 (Commits, GitHub, GitLab) Commit: b397c07108566c042aa2932ce12b6e58f79e109a
Dependencies: Stopgaps:

Status badges

Description

Currently, the construction for ehrhart_series and other related methods in backend_normaliz are not cached, which means that they need to be recreated every time they are called.

These functions should be cached.

Change History (20)

comment:1 Changed 2 years ago by mkoeppe

  • Milestone changed from sage-9.1 to sage-9.2

Batch modifying tickets that will likely not be ready for 9.1, based on a review of the ticket title, branch/review status, and last modification date.

comment:2 Changed 2 years ago by jipilab

  • Authors set to Jean-Philippe Labbé
  • Branch set to public/29196
  • Cc gh-kliem added
  • Commit set to 0a45ea8aab5d414faacf71500ea27df1d37c9a8f
  • Keywords ehrhart h star vector added
  • Status changed from new to needs_review

First version. Did I miss something?

New commits:

​b70549bFirst version of pickled ehrhart stuff
​0a45ea8pep8

comment:3 Changed 2 years ago by jipilab

  • Status changed from needs_review to needs_work

comment:4 Changed 2 years ago by jipilab

  • Work issues set to Failing doctests, hilbert series is not hashable

This is the first try. The tests are not passing. It needs some fixing.

comment:5 Changed 2 years ago by git

  • Commit changed from 0a45ea8aab5d414faacf71500ea27df1d37c9a8f to b894494ff997620b704ad582558b3b385b253b4b

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

​b894494make hilbert series arguments cachable

comment:6 Changed 2 years ago by gh-kliem

According to the documentation ​http://doc.sagemath.org/html/en/reference/misc/sage/misc/cachefunc.html, this should do it.

comment:7 Changed 2 years ago by gh-kliem

  • Authors changed from Jean-Philippe Labbé to Jean-Philippe Labbé, Jonathan Kliem
  • Branch changed from public/29196 to public/29196-reb
  • Commit changed from b894494ff997620b704ad582558b3b385b253b4b to 3ba66bd374dcfad3ba6214acfdd1df0b31322819
  • Status changed from needs_work to needs_review

New commits:

​db10c95Merge branch 'public/29196' of git://trac.sagemath.org/sage into public/29196-reb
​3ba66bdfixing doctests

comment:8 Changed 2 years ago by git

  • Commit changed from 3ba66bd374dcfad3ba6214acfdd1df0b31322819 to 6fa511dd53a95ba271f2a71b07214db8e8caddcb

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

​6fa511duse anonymous function for simplification

comment:9 Changed 2 years ago by gh-kliem

  • Branch changed from public/29196-reb to public/29196-reb2
  • Commit changed from 6fa511dd53a95ba271f2a71b07214db8e8caddcb to 721a1e714bfc2bf4ab540cc3abf7f9560afc6178

New commits:

​7bee618First version of pickled ehrhart stuff
​370c12cpep8
​c0d0d9duse key to make arguments cachable
​721a1e7fix doctest

comment:10 Changed 2 years ago by mkoeppe

Are the "work issues" resolved?

comment:11 Changed 2 years ago by gh-kliem

  • Work issues Failing doctests, hilbert series is not hashable deleted

Obviously :-) We have a green bot.

comment:12 Changed 2 years ago by gh-kliem

Well, ok, that doesn't mean anything if the bot doesn't have normaliz, which appears to be the case.

But I claim they are resolved anyway.

comment:13 Changed 2 years ago by mkoeppe

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

comment:14 Changed 2 years ago by gh-kliem

Thank you.

comment:15 Changed 2 years ago by vbraun

  • Status changed from positive_review to needs_work

If latte_int is installed: 

File "src/sage/geometry/polyhedron/base_QQ.py", line 547, in sage.geometry.polyhedron.base_QQ.Polyhedron_QQ.?
Failed example:
    Q.ehrhart_quasipolynomial.is_in_cache()         # optional - latte_int
Expected:
    True
Got:
    False
**********************************************************************
1 item had failures:
   1 of  67 in sage.geometry.polyhedron.base_QQ.Polyhedron_QQ.?
    [76 tests, 1 failure, 0.56 s]

comment:16 Changed 2 years ago by git

  • Commit changed from 721a1e714bfc2bf4ab540cc3abf7f9560afc6178 to b397c07108566c042aa2932ce12b6e58f79e109a

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

​b397c07fix doctest

comment:17 Changed 2 years ago by gh-kliem

  • Status changed from needs_work to needs_review

Sorry, I missed that. Didn't have latte_int installed on my laptop. You need to give is_in_cache the same arguments of course.

comment:18 Changed 2 years ago by mkoeppe

  • Status changed from needs_review to positive_review

comment:19 Changed 2 years ago by gh-kliem

Thank you.

comment:20 Changed 2 years ago by vbraun

  • Branch changed from public/29196-reb2 to b397c07108566c042aa2932ce12b6e58f79e109a
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.