Opened 3 years ago
Last modified 4 weeks ago
#22067 needs_work enhancement
generating function of integral points of polyhedra
Reported by:  dkrenn  Owned by:  

Priority:  major  Milestone:  sage9.1 
Component:  geometry  Keywords:  
Cc:  cheuberg, jipilab, ghbraunmath, slelievre  Merged in:  
Authors:  Daniel Krenn  Reviewers:  Matthias Koeppe 
Report Upstream:  N/A  Work issues:  
Branch:  u/dkrenn/gfpolyhedron (Commits)  Commit:  05108969d429ca7c6299f7b6d999f12e807ab224 
Dependencies:  #22065, #22066, #22803, #24837  Stopgaps: 
Description (last modified by )
Implement this using MacMahon's Omega operator (#22066).
Change History (71)
comment:1 Changed 3 years ago by
 Branch set to u/dkrenn/gfpolyhedron
comment:2 Changed 3 years ago by
 Commit set to 5d4910210e8c7c75d44c8aaa34c03f1d28b62909
 Component changed from PLEASE CHANGE to geometry
 Description modified (diff)
comment:3 Changed 3 years ago by
 Commit changed from 5d4910210e8c7c75d44c8aaa34c03f1d28b62909 to 32c6eeb9432da70febc8fe7d073f86023f7a1811
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
4b1c37e  references

79fe3b3  extend docstring at top of file

f1ec72e  Merge branch 'u/dkrenn/omega' of git://trac.sagemath.org/sage into t/22067/gfpolyhedron

ddbf737  repr_pretty and _latex_ of Hrepresentation

54fc5b8  repr_pretty_Hrepresentation for polyhedra

968a998  Merge branch 'u/dkrenn/inequalitypretty' of git://trac.sagemath.org/sage into t/22067/gfpolyhedron

1e327c5  add generating_function module to index

662195b  module description

4a98951  fix imports in doctests

32c6eeb  fix omega imports

comment:4 Changed 3 years ago by
 Cc cheuberg added
comment:5 Changed 3 years ago by
 Commit changed from 32c6eeb9432da70febc8fe7d073f86023f7a1811 to 7fcfe5ef1b679400b66fb6778473d692e20193e3
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
9ca1aeb  move repr_pretty code outside class, so that it can be used elsewhere as well

73649f9  Merge branch 'u/dkrenn/inequalitypretty' into t/22067/gfpolyhedron

6f59894  use repr_pretty (and remove old pretty_inequality)

ae030fa  more logging cleanup

a2334a3  make helper functions private

c4d01ec  make helper function private (part 2)

86f40ca  write missing docstrings/comments

24abfd4  todocleanup

76801db  fix a doctest

7fcfe5e  fix code typo

comment:6 Changed 3 years ago by
 Commit changed from 7fcfe5ef1b679400b66fb6778473d692e20193e3 to adf4dad4769aa2bc6f090b56c6a22e8279233523
Branch pushed to git repo; I updated commit sha1. New commits:
adf4dad  remove Summandization

comment:7 Changed 3 years ago by
 Commit changed from adf4dad4769aa2bc6f090b56c6a22e8279233523 to 999140f0b18a0b8d9fc9695316d0a16f6fde6dfa
comment:8 Changed 3 years ago by
 Status changed from new to needs_review
comment:9 Changed 3 years ago by
Return the generating function of the integer points of + the polyhedron's orthant with only nonnegative coordinates.
Why only the integer points with nonnegative coordinates?
comment:10 Changed 3 years ago by
 Commit changed from 999140f0b18a0b8d9fc9695316d0a16f6fde6dfa to f85e038c380e0686d06f2fe413ac22dbc95622e7
Branch pushed to git repo; I updated commit sha1. New commits:
f85e038  add doctest to _generate_mod_

comment:11 Changed 3 years ago by
 Commit changed from f85e038c380e0686d06f2fe413ac22dbc95622e7 to fce0c71d9bf117c3bfe549be7381a6008f2a8c9b
Branch pushed to git repo; I updated commit sha1. New commits:
f7e9540  fix doctest (conversion to symbolic)

82327c8  Merge branch 'u/dakrenn/laurentsubs' into t/22066/omega

16e515e  Merge branch 't/22066/omega' into t/22067/gfpolyhedron

0049218  remove periods at end of nonsentence in INPUTblock

91cab4a  Python3 print syntax

fce0c71  Merge branch 'u/dkrenn/inequalitypretty' into t/22067/gfpolyhedron

comment:12 Changed 3 years ago by
Merged in changed dependencies.
comment:13 followup: ↓ 17 Changed 3 years ago by
Hello,
This would be really useful.
1) The name is too generic: generating_function
could be anything (e.g. the number of lattice points in dilates). It should be something like non_negative_lattice_points_generating_function
.
2) What do you mean by "generating function"? Is it weighted by the sum of the vector?
3) You should at least check that the number of integer partitions is correctly computed by your function with the polyhedron
x1 >= x2 >= x3 >= x4 >= x5 >= 1
versus
sage: [Partitions(k, length=5).cardinality() for k in range(5,20)] [1, 1, 2, 3, 5, 7, 10, 13, 18, 23, 30, 37, 47, 57, 70]
comment:14 followup: ↓ 15 Changed 3 years ago by
I agree with Vincent that the name is too generic. This seems to compute what, for example, LattE would call the multivariate generating function (which can be computed by count multivariategeneratingfunction
after installing package latte_int
)  but not of the integer points of the given polyhedron but of its intersection with the positive orthant. This "positive orthant" business, I think, is just a peculiarity of the Omega approach but I don't think it's makes sense to expose it to the user. Maybe raise a NotImplementedError
instead if the given polytope is not contained in the positive orthant?
comment:15 in reply to: ↑ 14 ; followup: ↓ 18 Changed 3 years ago by
Replying to mkoeppe:
This "positive orthant" business, I think, is just a peculiarity of the Omega approach but I don't think it's makes sense to expose it to the user. Maybe raise a
NotImplementedError
instead if the given polytope is not contained in the positive orthant?
+1. The API would be much better (and allows extension in the future to count all points (that can already be done intersecting with the 2^{dim} orthants and applying symmetries)).
comment:16 Changed 3 years ago by
 Commit changed from fce0c71d9bf117c3bfe549be7381a6008f2a8c9b to 5d4018af485d1fdda08e9fb872a9a8a17579118f
Branch pushed to git repo; I updated commit sha1. New commits:
5c22d7f  rename methods (to make it more precise)

68e3fc7  check if polyhedron is in nonnegative orthant

d001a8e  doctests for polyhedra with negative coordinates

4ee76c4  fix existing doctests

7a96155  document nonnegativeorthantchanges

0afac56  minor rewrite (integer points > integral points)

871aea4  write down formula to make resulting GF precise

aa208ea  minor rewrite in SEEALSO blocks

594d9e5  example integer partitions

5d4018a  fix docs (ReST syntax)

comment:17 in reply to: ↑ 13 Changed 3 years ago by
Replying to vdelecroix:
This would be really useful.
Good :)
Thank you for your comments. Code updated.
1) The name is too generic:
generating_function
could be anything (e.g. the number of lattice points in dilates). It should be something likenon_negative_lattice_points_generating_function
.
It seems that "integral points" is used in sage.geometry.polyhedron` a couple of times, thus,
changed to generating_function_of_integral_points
.
2) What do you mean by "generating function"? Is it weighted by the sum of the vector?
Inserted a formula to make this precise.
3) You should at least check that the number of integer partitions is correctly computed by your function with the polyhedron
x1 >= x2 >= x3 >= x4 >= x5 >= 1versus
sage: [Partitions(k, length=5).cardinality() for k in range(5,20)] [1, 1, 2, 3, 5, 7, 10, 13, 18, 23, 30, 37, 47, 57, 70]
Indeed; example added.
comment:18 in reply to: ↑ 15 Changed 3 years ago by
Replying to vdelecroix:
Replying to mkoeppe:
This "positive orthant" business, I think, is just a peculiarity of the Omega approach but I don't think it's makes sense to expose it to the user. Maybe raise a
NotImplementedError
instead if the given polytope is not contained in the positive orthant?+1. The API would be much better (and allows extension in the future to count all points (that can already be done intersecting with the 2^{dim} orthants and applying symmetries)).
I agree. Changed the documentation and added a few lines to check if polyhedron is in the nonnegative orthant; otherwise raise NotImplementedError
. (Created a follow up ticket #22099 for the extension to arbitary corrdinates.)
comment:19 Changed 3 years ago by
 Summary changed from generating function of integervalued polyhedra to generating function of integral points of polyhedra
comment:20 Changed 3 years ago by
 Commit changed from 5d4018af485d1fdda08e9fb872a9a8a17579118f to fe77eec64d6d7ac53decb703977a1464c5d09bfa
Branch pushed to git repo; I updated commit sha1. New commits:
f046132  Python3 iteritems

0db5399  MPolynomial: correct coefficient ring when dict as input

a20fb8a  revert 0f88065556 quickfix in normalize

397301a  doctest example of ticket 21999

fceb3e1  Merge branch 't/21999/laurentfloorhash' into t/22066/omega

fe77eec  Merge branch 't/22066/omega' into t/22067/gfpolyhedron

comment:21 followup: ↓ 23 Changed 3 years ago by
 Status changed from needs_review to needs_work
sage: P = Polyhedron(vertices=[[1], [5]]) sage: P.generating_function_of_integral_points() TypeError: unsupported operand type(s) for : 'tuple' and 'tuple'
comment:22 Changed 3 years ago by
 Commit changed from fe77eec64d6d7ac53decb703977a1464c5d09bfa to 35cf3a2c59178cb55df897b9eaf3d4a6d7beafe1
Branch pushed to git repo; I updated commit sha1. New commits:
35cf3a2  fix 1dimensial case (creation of univariate laurent polynomial ring)

comment:23 in reply to: ↑ 21 Changed 3 years ago by
 Status changed from needs_work to needs_review
Replying to mkoeppe:
sage: P = Polyhedron(vertices=[[1], [5]]) sage: P.generating_function_of_integral_points() TypeError: unsupported operand type(s) for : 'tuple' and 'tuple'
Fixed.
comment:24 followup: ↓ 29 Changed 3 years ago by
I think the name of the variables used in the generating function should be a user option
comment:25 followup: ↓ 30 Changed 3 years ago by
sage: P = Polyhedron(rays=[(1, sqrt(2)), (0, 1)]) sage: P.generating_function_of_integral_points() 1 * (y0 + 1)^1 * (y1 + 1)^1
I had hoped for an error
comment:26 followups: ↓ 31 ↓ 32 ↓ 33 Changed 3 years ago by
Also I think the method generating_function_of_integral_points
should have an algorithm
keyword. For polytopes, could provide a naive algorithm that just collects the monomials corresponding to integral_points
. And another algorithm would be to just call LattE.
Various algorithms options such as result_as_tuple
are not sufficiently documented.
For the LattE algorithm, the best output format would be a sum of Factorizations. Is this what result_as_tuple
can represent?
comment:27 Changed 3 years ago by
 Reviewers set to Matthias Koeppe
 Status changed from needs_review to needs_work
comment:28 Changed 3 years ago by
 Commit changed from 35cf3a2c59178cb55df897b9eaf3d4a6d7beafe1 to 401c4988cf876724d7d3d6079c11f35c37c06fa6
Branch pushed to git repo; I updated commit sha1. New commits:
98d46b6  new option: specify the prefix of the output variables

fb2f8f6  test and document prefix_variable_name

7bbae4b  extend documentation of parameter result_as_tuple

3d9bb3f  extend documentation of parameter sort_factors

401c498  check base ring of the polyhedron (if it is ZZ or QQ)

comment:29 in reply to: ↑ 24 Changed 3 years ago by
Replying to mkoeppe:
I think the name of the variables used in the generating function should be a user option
True; option is now available :)
comment:30 in reply to: ↑ 25 Changed 3 years ago by
Replying to mkoeppe:
sage: P = Polyhedron(rays=[(1, sqrt(2)), (0, 1)]) sage: P.generating_function_of_integral_points() 1 * (y0 + 1)^1 * (y1 + 1)^1I had hoped for an error
Now the base ring of the polyhedron is inspected.
comment:31 in reply to: ↑ 26 ; followup: ↓ 35 Changed 3 years ago by
Replying to mkoeppe:
Various algorithms options such as
result_as_tuple
are not sufficiently documented.
Docs extended.
comment:32 in reply to: ↑ 26 Changed 3 years ago by
Replying to mkoeppe:
Also I think the method
generating_function_of_integral_points
should have analgorithm
keyword. For polytopes, could provide a naive algorithm that just collects the monomials corresponding tointegral_points
. And another algorithm would be to just call LattE.
As this does not change the interface, only extends it, I have put this on the followup enhancement ticket #22111.
comment:33 in reply to: ↑ 26 Changed 3 years ago by
Replying to mkoeppe:
For the LattE algorithm, the best output format would be a sum of Factorizations. Is this what
result_as_tuple
can represent?
Yes, correct. (As mentioned, I've extended the docstring of this option.)
comment:34 Changed 3 years ago by
 Status changed from needs_work to needs_review
comment:35 in reply to: ↑ 31 ; followup: ↓ 36 Changed 3 years ago by
comment:36 in reply to: ↑ 35 Changed 3 years ago by
Replying to mkoeppe:
Replying to dkrenn:
Replying to mkoeppe:
Various algorithms options such as
result_as_tuple
are not sufficiently documented.Docs extended.
Wouldn't it be nicer to have a class to represent such sums, much like Factorization represents products?
Yes, indeed. I had this (a class Summandization
in some draft, but as it is nowhere else in SageMath (e.g. partial_fraction_decomposition
), I decided to keep it simple and consistent and use a tuple.
comment:37 Changed 3 years ago by
 Cc jipilab added
comment:38 Changed 3 years ago by
 Commit changed from 401c4988cf876724d7d3d6079c11f35c37c06fa6 to 22bd0c4c8e5cb0a2829867e9c069561872b39879
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
b8314a3  Trac #22066: Format Mac1915

2ce3ab1  Trac #22066: minor rewording

4b1bb1d  Merge remotetracking branch 'trac/u/cheuberg/omega7.5.rc1' into t/22066/omega7.5.rc1

fded046  Merge branch 'u/dkrenn/laurentuniconvert' in 7.5.1

c75d370  trac 21855 some details

11707ed  trac 21855 more details

65e2fc5  Merge tag '7.6.beta2' into t/21855/public/21855

80eea43  Merge branch 'public/21855' of git://trac.sagemath.org/sage into t/21976/laurentefficientconstruction

f1e2fcd  Merge branch 'u/dkrenn/laurentefficientconstruction' of git://trac.sagemath.org/sage into t/22066/omega7.5.rc1

22bd0c4  Merge branch 'u/dkrenn/omega7.5.rc1' of git://trac.sagemath.org/sage into t/22067/gfpolyhedron

comment:39 followup: ↓ 40 Changed 3 years ago by
 Commit changed from 22bd0c4c8e5cb0a2829867e9c069561872b39879 to 35507829141ae344a84be1ce67d557c99087db0a
comment:40 in reply to: ↑ 39 Changed 3 years ago by
comment:41 Changed 3 years ago by
 Commit changed from 35507829141ae344a84be1ce67d557c99087db0a to 4d2ccb6f12eb183daebe9c4e046ecb0be7ac6f74
Branch pushed to git repo; I updated commit sha1. New commits:
4d2ccb6  unify INPUT/OUTPUT blocks

comment:42 followup: ↓ 45 Changed 3 years ago by
needs rebasing
comment:43 Changed 3 years ago by
 Milestone changed from sage7.5 to sage7.6
 Status changed from needs_review to needs_work
comment:44 Changed 3 years ago by
 Commit changed from 4d2ccb6f12eb183daebe9c4e046ecb0be7ac6f74 to 3a3aec5122c35e5fc9291b69895f6e185a54cc41
Branch pushed to git repo; I updated commit sha1. New commits:
cf22ee8  Merge branch 'u/dkrenn/laurentefficientconstruction' of git://trac.sagemath.org/sage into public/polynomials/multivariable_laurent_poly_constructor21976

c58559d  Some reviewer changes.

1b7377c  Merge branch 'develop' into public/polynomials/laurent_mpoly_constructor21976

3a3aec5  Merge branch 'public/polynomials/laurent_mpoly_constructor21976' of git://trac.sagemath.org/sage into t/22067/gfpolyhedron

comment:45 in reply to: ↑ 42 Changed 3 years ago by
 Status changed from needs_work to needs_review
Replying to mkoeppe:
needs rebasing
This was because of a change in one of its dependencies. Merged in; should now work again.
comment:46 Changed 3 years ago by
 Commit changed from 3a3aec5122c35e5fc9291b69895f6e185a54cc41 to b09d1622689b7c7c35a3413698f222177e9e6876
Branch pushed to git repo; I updated commit sha1. New commits:
b09d162  Trac #22067: minor modification of logging output

comment:47 Changed 3 years ago by
 Commit changed from b09d1622689b7c7c35a3413698f222177e9e6876 to 423096c71bac2a2aa4c75311baca4c0a4c929bf9
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
24a861f  rename _generate_mods_ to generate_mods

7bb6fef  adapt to previous code movement

fd117ff  factor out callingOmegacode

f137fba  small cleanup

aaf0fa0  restrict splitting off inequality to constants <= 0

802c523  use _simplify_ to cancel terms

2d94245  update doctests

b4082d2  function _simplify_ for cancelling factors in intermediate result (quotient)

b02b7f1  use and test new feature in _Omega_function

423096c  Merge branch 'u/dakrenn/omegasimplify' into t/22067/gfpolyhedron

comment:48 Changed 3 years ago by
 Dependencies changed from #22065, #22066 to #22065, #22066, #22803
 Status changed from needs_review to needs_work
 Merged in 7.6, as some error messages concerning polynomial rings have changed and thus doctests failed.
 Merged in #22803, which simplifies the some generating functions due to cancellations
 Fixed a small bug.
 Restructured/refactored some code to make testing the overall result easier.
 Extended the existing doctests to see the correctness of the results.
(Set it to needs work as some docstrings are not yet written.)
comment:49 Changed 3 years ago by
 Commit changed from 423096c71bac2a2aa4c75311baca4c0a4c929bf9 to 3cfd809279a92f1259e01381125afa0e8965e97d
comment:50 Changed 3 years ago by
 Status changed from needs_work to needs_review
comment:51 Changed 3 years ago by
2 failing doctests, see patchbot report (wrong import)
comment:52 followup: ↓ 54 Changed 3 years ago by
 Status changed from needs_review to needs_work
ping ? just 2 imports need to be changed..
comment:53 Changed 3 years ago by
 Commit changed from 3cfd809279a92f1259e01381125afa0e8965e97d to c91baa5d0ba1d7febd8cd5afe9c92b724c52efc2
Branch pushed to git repo; I updated commit sha1. New commits:
c91baa5  Trac #22067: correct import of lcm

comment:54 in reply to: ↑ 52 Changed 3 years ago by
Replying to chapoton:
ping ? just 2 imports need to be changed..
Sorry, I am currently some kind of busy. However, changed the import. Let's see what the patchbot says now ;)
comment:55 Changed 3 years ago by
 Status changed from needs_work to needs_review
comment:56 followup: ↓ 60 Changed 3 years ago by
Trivial remark: some "laurent" are missing their capitals.
comment:57 followup: ↓ 61 Changed 2 years ago by
 Status changed from needs_review to needs_work
and one failing doctest
comment:58 Changed 17 months ago by
 Commit changed from c91baa5d0ba1d7febd8cd5afe9c92b724c52efc2 to 1b922e779cf2e4e32a0257ed39f63fdcc5c09bdb
comment:59 Changed 17 months ago by
 Commit changed from 1b922e779cf2e4e32a0257ed39f63fdcc5c09bdb to 0f2d193c0a27c2073c9f149c47a9422c680ded66
Branch pushed to git repo; I updated commit sha1. New commits:
0f2d193  doc: laurent > Laurent

comment:60 in reply to: ↑ 56 Changed 17 months ago by
comment:61 in reply to: ↑ 57 Changed 17 months ago by
comment:62 Changed 17 months ago by
 Dependencies changed from #22065, #22066, #22803 to #22065, #22066, #22803, #24837
 Status changed from needs_work to needs_review
comment:63 Changed 17 months ago by
you can just remove the dependencies (empty this field), once closed
comment:64 Changed 17 months ago by
 Commit changed from 0f2d193c0a27c2073c9f149c47a9422c680ded66 to 372da68378a9fce88ff20f1308bd243bad28c396
Branch pushed to git repo; I updated commit sha1. New commits:
372da68  cleanup pyflakes warnings

comment:65 Changed 17 months ago by
 Commit changed from 372da68378a9fce88ff20f1308bd243bad28c396 to 1583e91e3f207b0ce47369cd00083c963889dba7
Branch pushed to git repo; I updated commit sha1. New commits:
1583e91  fixup: forgotten ** left of kwds

comment:66 Changed 16 months ago by
 Commit changed from 1583e91e3f207b0ce47369cd00083c963889dba7 to 05108969d429ca7c6299f7b6d999f12e807ab224
Branch pushed to git repo; I updated commit sha1. New commits:
0510896  uniquify sorting in SplitOffSimpleInequalities

comment:67 Changed 16 months ago by
 Cc ghbraunmath added
comment:68 Changed 11 months ago by
 Cc slelievre added
 Description modified (diff)
 Milestone changed from sage7.6 to sage8.8
comment:69 Changed 7 months ago by
 Milestone changed from sage8.8 to sage8.9
Moving tickets from the Sage 8.8 milestone that have been actively worked on in the last six months to the next release milestone (optimistically).
comment:70 Changed 4 months ago by
 Status changed from needs_review to needs_work
one failing doctest.
comment:71 Changed 4 weeks ago by
 Milestone changed from sage8.9 to sage9.1
Ticket retargeted after milestone closed
Last 10 new commits:
allow passing mon in _element_constructor_
fix pickling bug
determine parent at top of _element_constructor_
rewrite to detect more specialized situations
Merge branch 'u/dkrenn/laurentefficientconstruction' of trac.sagemath.org:sage into u/dakrenn/omega
quickfix in normalize
Merge branch 'u/dkrenn/laurentfloorhash' of trac.sagemath.org:sage into u/dakrenn/omega
use __call__ in subs to be more efficient
Merge branch 'u/dkrenn/laurentsubs' of trac.sagemath.org:sage into u/dakrenn/omega
Merge branch 'u/dakrenn/omega' into u/dakrenn/gfpolyhedron