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.

Sign up for GitHub

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

Jump to bottom

Hs/hilb #787

Merged
merged 7 commits into from
Mar 8, 2024
Merged

Hs/hilb #787

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9a3ee00
ssue 2411 (int overflow in hilb computation)
hannes14 Feb 23, 2024
4098d2e
also variant with weights, added test
hannes14 Feb 23, 2024
501539e
add hilbert_series_data
hannes14 Feb 28, 2024
14284c1
conversion to Vector{BigInt} for hilbert_series_data
hannes14 Feb 28, 2024
ca8b21d
Update src/ideal/ideal.jl
hannes14 Mar 1, 2024
7166bf9
Update test/ideal/sideal-test.jl
hannes14 Mar 1, 2024
9daf752
Update src/ideal/ideal.jl
hannes14 Mar 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions deps/src/ideals.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -713,10 +713,12 @@ void singular_define_ideals(jlcxx::Module & Singular)
Singular.method("scHilb", [](ideal I, ring r, jlcxx::ArrayRef<int> a) {
const ring origin = currRing;
rChangeCurrRing(r);
intvec * v = hFirstSeries(I, NULL, r->qideal);
int * content = v->ivGetVec();
bigintmat * v = hFirstSeries0b(I, r->qideal, NULL,NULL, r,coeffs_BIGINT);
for (int j = 0; j < v->length(); j++)
a.push_back(content[j]);
{
number n=(*v)[j];
a.push_back(n_Int(n,coeffs_BIGINT));
hannes14 marked this conversation as resolved.
Show resolved Hide resolved
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

}
delete v;
rChangeCurrRing(origin);
});
Expand All @@ -725,12 +727,12 @@ void singular_define_ideals(jlcxx::Module & Singular)
intvec * w = to_intvec(weights);
const ring origin = currRing;
rChangeCurrRing(r);
intvec * v = hFirstSeries(I, NULL, r->qideal, w);
bigintmat * v = hFirstSeries0b(I, r->qideal, w, NULL,r,coeffs_BIGINT);
delete w;
int * content = v->ivGetVec();
for (int j = 0; j < v->length(); j++)
{
a.push_back(content[j]);
number n=(*v)[j];
a.push_back(n_Int(n,coeffs_BIGINT));
}
delete v;
rChangeCurrRing(origin);
Expand All @@ -742,13 +744,13 @@ void singular_define_ideals(jlcxx::Module & Singular)
intvec * sh = to_intvec(shifts);
const ring origin = currRing;
rChangeCurrRing(r);
intvec * v = hFirstSeries(I, sh, r->qideal, w);
bigintmat * v = hFirstSeries0b(I, r->qideal, w, sh,r,coeffs_BIGINT);
delete sh;
delete w;
int * content = v->ivGetVec();
for (int j = 0; j < v->length(); j++)
{
a.push_back(content[j]);
number n=(*v)[j];
a.push_back(n_Int(n,coeffs_BIGINT));
}
delete v;
rChangeCurrRing(origin);
Expand Down
4 changes: 4 additions & 0 deletions test/ideal/sideal-test.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,10 @@ end
I=Ideal(R,[x,y,z])
I=std(I)
@test hilbert_series(I,Qt) == -t^3+3*t^2-3*t+1
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.

end

@testset "sideal.oscar#1702" begin
Expand Down
Loading