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

Wrong computation of Hilbert polynomials #33597

Closed
loefflerd mannequin opened this issue Mar 30, 2022 · 18 comments
Closed

Wrong computation of Hilbert polynomials #33597

loefflerd mannequin opened this issue Mar 30, 2022 · 18 comments

Comments

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Mar 30, 2022

sage: R.<X, Y, Z> = QQ[]
sage: I = R.ideal([X^2*Y^3, X*Z])
sage: I.hilbert_polynomial()
-t - 5

The Hilbert polynomial, by definition, has to take non-negative integer values at all sufficiently large integers t, so this computation can't possibly be right. The correct answer is t + 5, which is what one gets with the algorithm='singular' option.

Inspecting the code, it looks like the denominator of the Hilbert series is getting normalised wrongly in some cases.

Component: commutative algebra

Author: Frédéric Chapoton, Kwankyu Lee

Branch/Commit: 98f118d

Reviewer: Kwankyu Lee, Frédéric Chapoton

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

@loefflerd loefflerd mannequin added this to the sage-9.6 milestone Mar 30, 2022
@loefflerd

This comment has been minimized.

@loefflerd

This comment has been minimized.

@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 May 3, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Sep 19, 2022
@fchapoton
Copy link
Contributor

Branch: u/chapoton/33597

@fchapoton
Copy link
Contributor

Author: Frédéric Chapoton

@fchapoton
Copy link
Contributor

New commits:

51cb095fixes in Hilbert polynomial

@fchapoton
Copy link
Contributor

Commit: 51cb095

@fchapoton
Copy link
Contributor

comment:6

bot is morally green, so please review

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 30, 2022

Changed branch from u/chapoton/33597 to u/klee/33597

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 30, 2022

Reviewer: Kwankyu Lee

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 30, 2022

comment:8

Sorry for hijacking this ticket. I made a few modifications to the patch so that

(1) we do not assume denom[0] can be only -1.

(2) the code is a bit more efficient.

Before your patch

sage: R.<X, Y, Z> = QQ[]
sage: I = R.ideal([X^2*Y^3, X*Z])
sage: I.hilbert_polynomial() 
sage: I.hilbert_polynomial()
-t - 5
sage: timeit('I.hilbert_polynomial()')
625 loops, best of 3: 409 μs per loop
sage: timeit('I.hilbert_polynomial()')
625 loops, best of 3: 416 μs per loop

After your patch

sage: R.<X, Y, Z> = QQ[]
sage: I = R.ideal([X^2*Y^3, X*Z])
sage: I.hilbert_polynomial()
t + 5
sage: timeit('I.hilbert_polynomial()')
625 loops, best of 3: 299 μs per loop
sage: timeit('I.hilbert_polynomial()')
625 loops, best of 3: 298 μs per loop

After my modifications

sage: R.<X, Y, Z> = QQ[]
sage: I = R.ideal([X^2*Y^3, X*Z])
sage: I.hilbert_polynomial()
t + 5
sage: timeit('I.hilbert_polynomial()')
625 loops, best of 3: 230 μs per loop
sage: timeit('I.hilbert_polynomial()')
625 loops, best of 3: 235 μs per loop

I am positive to the patch.

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 30, 2022

Changed commit from 51cb095 to b8630ac

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 30, 2022

Changed commit from b8630ac to 98f118d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 30, 2022

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

98f118dMinor edits

@fchapoton
Copy link
Contributor

comment:10

ok, good to go, thanks

@fchapoton
Copy link
Contributor

Changed reviewer from Kwankyu Lee to Kwankyu Lee, Frédéric Chapoton

@fchapoton
Copy link
Contributor

Changed author from Frédéric Chapoton to Frédéric Chapoton, Kwankyu Lee

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 30, 2022

comment:11

Thank you!

@vbraun
Copy link
Member

vbraun commented Oct 11, 2022

Changed branch from u/klee/33597 to 98f118d

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