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

Add mstd, divrem back #628

Merged
merged 1 commit into from
Apr 16, 2023
Merged

Add mstd, divrem back #628

merged 1 commit into from
Apr 16, 2023

Conversation

fingolfin
Copy link
Member

Adds back the changes from PR #625

@fingolfin fingolfin requested a review from hannes14 March 21, 2023 07:09
@hannes14
Copy link
Member

hannes14 commented Mar 21, 2023

The problem is the handling of a special case in depth: depth(I, F) is infinity (reported as -1) if F in I*F is.
This is tested in src/Modules/homological-algebra.jl:456: if is_zero(F) || is_equal((I*F)[1], F) return -1 end
but also in Singular (Singular/LIB/homolog.lib(1784)): if (size(reduce(M,std(I*M),5))==0) {return (-1); }
https://github.com/oscar-system/Oscar.jl/blob/master/src/Modules/homological-algebra.jl#L458
continues:

 MX = singular_module(F)
 SG = Singular.Module(base_ring(MX), MX(zero(F)))
 singular_assure(I)
 return Singular.LibHomolog.depth(SG, I.gens.S)

which results in a zero-module passed to Singular, which triggers the test above.
Possible solutions:

  • remove the special case test from homolog.lib and build a new Singular_jll or
  • change the Oscar part above to:
     MX = singular_module(F)
     SG = Singular.Module(base_ring(MX), MX(F))
     singular_assure(I)
     return Singular.LibHomolog.depth(SG, I.gens.S)

@wdecker

@fingolfin
Copy link
Member Author

@hannes14 @wdecker so what's the status of this? IIRC the plan was to adjust Singular again, then update Singular_jll, then we can merge this PR? Or what else do we do?

@hannes14
Copy link
Member

see JuliaPackaging/Yggdrasil#6577
this is Singular 4.3.2 with changes to function "depth"

@fingolfin
Copy link
Member Author

I've tested this locally and there it seems to be fine now:

julia> R, (x, y, z) = polynomial_ring(QQ, ["x", "y", "z"]);

julia> F = free_module(R, 1);

julia> I = ideal(R, [x*z-z, x*y-y, x])
ideal(x*z - z, x*y - y, x)

julia> depth(I, F)
3

Let's wait and see what the OscarCI tests say.

Copy link
Member

Choose a reason for hiding this comment

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

works with Singular_jll "403.201.000" and "403.201.001", but if we want to avoid the problems with "depth",
we need "403.201.001". libsingular_julia_jll should be "0.30.0", but this refers to Singular_jll "403.201.000"

  • do we need a new libsingular_julia_jll with only this dependency changed?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think we need an libsingular_julia_jll update

@fingolfin fingolfin merged commit 0ed9c79 into master Apr 16, 2023
@fingolfin fingolfin deleted the mstd-divrem branch April 16, 2023 17:18
fingolfin added a commit to fingolfin/Singular.jl that referenced this pull request Jun 6, 2023
Co-authored-by: Hans Schoenemann <hannes@mathematik.uni-kl.de>
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.

2 participants