Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug in h_star_vector for polytopes with the normaliz backend #33847

Closed
yuan-zhou opened this issue May 12, 2022 · 13 comments
Closed

Bug in h_star_vector for polytopes with the normaliz backend #33847

yuan-zhou opened this issue May 12, 2022 · 13 comments

Comments

@yuan-zhou
Copy link

Dr. Ben Braun reported the following error.

The h-star-vector records the coefficients of the polynomial in the
numerator of the Ehrhart series of a lattice polytope.
The method h_star_vector() for polytopes with the normaliz backend was implemented in #28413.
The implementation seems to have an error, namely that it drops internal zeros from the vector.

Example:

sage: L = [[1, 0, 0, 0, 0, 0], [1, 1, 0, 0, 0, 0], [1, 0, 1, 0, 0, 0], [1, 0, 0,
....:  1, 0, 0], [1, 0, 0, 0, 1, 0], [1, 0, 0, 1, 2, 3]]
sage: P = Polyhedron(vertices=L,backend='normaliz')
sage: P.ehrhart_series().numerator()
2*t^2 + 1
sage: P.h_star_vector()
[1, 2]

Actually, the correct return should be [1, 0, 2].

This bug is caused by the line 1437 of src/sage/geometry/polyhedron/backend_normaliz.py, which forgets to pass sparse=False into self.ehrhart_series().numerator().coefficients().

We fix the bug by changing this line to
return self.ehrhart_series().numerator().list().

CC: @braunmath @sophiasage @jplab @fchapoton @w-bruns @videlec @mkoeppe

Component: geometry

Keywords: polytopes, normaliz

Author: Yuan Zhou

Branch/Commit: 6027fb1

Reviewer: Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/33847

@yuan-zhou yuan-zhou added this to the sage-9.7 milestone May 12, 2022
@yuan-zhou

This comment has been minimized.

@yuan-zhou
Copy link
Author

@yuan-zhou
Copy link
Author

New commits:

0a08dfbreturn all coefficients, including the zero coefficients of P.ehrhart_series().numerator()

@yuan-zhou
Copy link
Author

Commit: 0a08dfb

@mkoeppe
Copy link
Member

mkoeppe commented May 13, 2022

comment:4

Output is missing in the new doctest

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 13, 2022

Changed commit from 0a08dfb to 6027fb1

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 13, 2022

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

6027fb1new doctest

@fchapoton
Copy link
Contributor

comment:7

please fill the "Authors" box

@yuan-zhou
Copy link
Author

Author: Yuan Zhou

@yuan-zhou
Copy link
Author

comment:9

Patchbot reports one doctest failure, as follows. I'm not sure if it is related.

File "sage/parallel/map_reduce.py", line 1151, in sage.parallel.map_reduce.RESetMapReduce.start_workers
Failed example:
    all(not w.is_alive() for w in S._workers)
Expected:
    True
Got:
    False

@mkoeppe
Copy link
Member

mkoeppe commented May 13, 2022

comment:10

unrelated, that's #33834

@mkoeppe
Copy link
Member

mkoeppe commented May 13, 2022

Reviewer: Matthias Koeppe

@vbraun
Copy link
Member

vbraun commented May 24, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants