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

Hs/hilb #787

Merged
merged 7 commits into from
Mar 8, 2024
Merged

Hs/hilb #787

merged 7 commits into from
Mar 8, 2024

Conversation

hannes14
Copy link
Member

see oscar-system/Oscar.jl#2411,
but to really solve it, HilbertData must be changed to Vector{Integer} or similiar

@hannes14
Copy link
Member Author

@wdecker @fingolfin @JohnAAbbott
HilbertData is Vector{Int32} which causes this overflow, the algorithm in Singular works with GMP integers.

a.push_back(content[j]);
{
number n=(*v)[j];
a.push_back(n_Int(n,coeffs_BIGINT));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if n does not fit into an int, what happens then? Is an exception thrown (where is it caught), or does it again just silently overflow?

Can't we just return an array of bigints? That requires changing a to ArrayRef<jl_value_t>, I think, and then putting n_Z or BigInt values in there?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Singulkar prints an error message and return 0, if the conversion to int fails

src/ideal/ideal.jl Outdated Show resolved Hide resolved
function hilbert_series_data(I::sideal{spoly{T}}) where T <: Nemo.FieldElem
Qt,(t,) = polynomial_ring(ZZ, ["t"])
h = hilbert_series(I,Qt)
v = [convert(BigInt,c) for c in coefficients(h)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but as long as the coefficients in h are ints, this is pointless, isn't it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the polynomial h is in the ring Qt, i.e. its coefficients are in ZZ

src/ideal/ideal.jl Outdated Show resolved Hide resolved
test/ideal/sideal-test.jl Outdated Show resolved Hide resolved
@@ -1468,6 +1469,37 @@ function hilbert_series(I::sideal{spoly{T}}, w::Vector{<:Integer}, Qt::PolyRing)
return Qt(new_ptr)
end

@doc raw"""
hilbert_series_data(I::sideal{spoly{T}}) where T <: Nemo.FieldElem
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is a new function needed? Can't we just modify the existing hilbert_series functions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without a second name Oscar-tests would fail: I do not know how to change Oscar and Singular..jl in one step.

A,x = polynomial_ring(QQ,["x$i" for i in 1:37])
I = Ideal(A,[2*x[11] - 2*x[17] - 2*x[24] + 2*x[32] - 111916*x[37], 2*x[4] - 2*x[8] - 2*x[26] + 2*x[34] - 41216*x[37], 2*x[2] - 2*x[9] - 2*x[20] + 2*x[35] + 37974*x[37], x[28] - x[36], x[21] - x[36], x[27] - x[28] + x[33] + x[36], x[26] - x[27] - x[33] + x[34], x[20] - x[21] + x[35] + x[36], x[15] - x[21] - x[28] + x[36], x[10] - x[36], x[25] - x[28] + x[31] + x[36], x[24] - x[25] - x[26] + x[27] - x[31] + x[32] + x[33] - x[34], -x[14] + x[15] + x[18] - x[21] + x[25] - x[28] + x[31] + x[36], x[13] - x[14] + x[18] - x[19] - 2*x[20] + 2*x[21] - x[26] + x[27] + x[33] - x[34] - 2*x[35] - 2*x[36], x[9] - x[10] + x[35] + x[36], x[6] - x[10] - x[28] + x[36], x[19] - x[21] + x[30] + x[36], -x[18] + x[19] + x[23] - x[25] - x[27] + x[28] + x[30] - x[31] - x[33] - x[36], x[17] - x[19] - x[30] + x[32], x[12] - x[14] - x[17] + x[18] - x[27] + x[28] + x[31] - x[32] - x[33] - x[36], x[8] - x[10] + x[34] + x[36], x[5] - x[6] - x[8] + x[10] - x[27] + x[28] - x[34] - x[36], x[3] - x[10] - x[21] + x[36], -x[18] + x[19] + x[20] - x[21] + x[29] + x[30] + x[35] + x[36], x[22] + x[23] + x[24] - x[25] - x[29] - x[30] - x[31] + x[32], x[16] + x[17] + x[18] - x[19] - x[22] - x[23] - x[24] + x[25], x[11] + x[12] + x[13] - x[14] - x[16] - x[17] - x[18] + x[19] + x[22] + x[23] + x[24] - x[25] + x[29] + x[30] + x[31] - x[32], x[7] + x[8] + x[9] - x[10] - x[33] + x[34] + x[35] + x[36], x[4] + x[5] + x[9] - x[10] + x[26] - x[27] + x[35] + x[36], x[2] + x[3] + x[9] - x[10] + x[20] - x[21] + x[35] + x[36], x[1] - x[3] - x[6] + x[10] - x[15] + x[21] + x[28] - x[36], -x[27]*x[36] + x[34]*x[35], -x[25]*x[36] + x[32]*x[35], x[14]*x[36] + x[19]*x[35] + x[25]*x[36] + x[27]*x[36] - x[32]*x[35] - x[34]*x[35], -x[19]*x[36] - x[25]*x[36] + x[32]*x[34] + x[32]*x[35], -x[19]*x[35] - x[19]*x[36] + x[25]*x[34] - x[25]*x[36] + x[32]*x[34] + x[32]*x[35], x[14]*x[36] - x[19]*x[35] + x[25]*x[34] + x[27]*x[32], x[14]*x[35] - x[14]*x[36] + x[19]*x[35] - x[19]*x[36] + x[25]*x[27] - x[25]*x[34] - x[27]*x[32] + x[32]*x[34], x[14]*x[34] + x[19]*x[27] - 2*x[19]*x[35] + 2*x[25]*x[34] - x[25]*x[36] + x[32]*x[35], x[14]*x[32] - 2*x[14]*x[36] + x[19]*x[25] - 2*x[19]*x[35] - x[27]*x[36] + x[34]*x[35]])
I=std(I)
@test hilbert_series(I,Qt)==-t^37 + 31*t^36 - 456*t^35 + 4200*t^34 - 26775*t^33 + 121737*t^32 - 376992*t^31 + 556512*t^30 + 1739100*t^29 - 16811300*t^28 + 75314624*t^27 - 246484224*t^26 + 650872404*t^25 - 1444243500*t^24 + 2750940000*t^23 - 4555556640*t^22 + 6611884290*t^21 - 8454204990*t^20 + 9552774000*t^19 - 9552774000*t^18 + 8454204990*t^17 - 6611884290*t^16 + 4555556640*t^15 - 2750940000*t^14 + 1444243500*t^13 - 650872404*t^12 + 246484224*t^11 - 75314624*t^10 + 16811300*t^9 - 1739100*t^8 - 556512*t^7 + 376992*t^6 - 121737*t^5 + 26775*t^4 - 4200*t^3 + 456*t^2 - 31*t + 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether we can come up with an example where the coefficients of the series don't fit into 64bit, then we could also handle that error situation.

Anyway, I gather this is the example from the Oscar issue. There, the ultimate failure is in a degree computation. Is that fixed now? Cn we test this here, too?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reminder link to the original issue oscar-system/Oscar.jl#2411 is in the first message.
It is easy to find examples with bigger coefficients by using more variables; I would guess about 70 variables suffice, maybe fewer.

hannes14 and others added 3 commits March 1, 2024 09:50
Co-authored-by: Max Horn <max@quendi.de>
Co-authored-by: Max Horn <max@quendi.de>
Co-authored-by: Max Horn <max@quendi.de>
@fingolfin fingolfin merged commit 9413c63 into oscar-system:master Mar 8, 2024
12 of 14 checks passed
hannes14 added a commit to hannes14/Singular.jl that referenced this pull request Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants