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

Oscar Singular coercion ambiguities #976

Open
thofma opened this issue Jan 12, 2022 · 9 comments · May be fixed by #4342
Open

Oscar Singular coercion ambiguities #976

thofma opened this issue Jan 12, 2022 · 9 comments · May be fixed by #4342
Labels
singular coding sprint triage

Comments

@thofma
Copy link
Collaborator

thofma commented Jan 12, 2022 •
edited
Loading 

julia> R, s = Oscar.polynomial_ring(Oscar.QQ, "s")
(Univariate polynomial ring in s over QQ, s)

julia> K, q = Oscar.number_field(s^2 - 2, "q")
(Number field of degree 2 over QQ, q)

julia> Kgnd, a1 = Oscar.rational_function_field(K, "a1")
(Rational function field over K, a1)

julia> PolRng, (a2, a3, a4) = Oscar.polynomial_ring(Kgnd, ["a2", "a3", "a4"])
(Multivariate polynomial ring in 3 variables over Kgnd, AbstractAlgebra.Generic.MPoly{AbstractAlgebra.Generic.RationalFunctionFieldElem{AbsSimpleNumFieldElem, AbstractAlgebra.Generic.Poly{AbsSimpleNumFieldElem}}}[a2, a3, a4])

julia> glist = [
             a1^3*a3 - 2*q*a1^2*a2^2 - 5*a1*a2^2*a3 + 3*a1*a3^3,
             a1^2*a2*a3 - 2*q*a1*a2^3 - 5*a2^3*a3 + 3*a2*a3^3,
             2*a1*a2*a3*a4 - a1*a3^3 - 2*q*a2^3*a4 + a2*a3^3
         ]
3-element Vector{AbstractAlgebra.Generic.MPoly{AbstractAlgebra.Generic.RationalFunctionFieldElem{AbsSimpleNumFieldElem, AbstractAlgebra.Generic.Poly{AbsSimpleNumFieldElem}}}}:
 -5*a1*a2^2*a3 - 2*q*a1^2*a2^2 + 3*a1*a3^3 + a1^3*a3
 -5*a2^3*a3 - 2*q*a1*a2^3 + 3*a2*a3^3 + a1^2*a2*a3
 -2*q*a2^3*a4 + a2*a3^3 + 2*a1*a2*a3*a4 - a1*a3^3

julia> gidl = Oscar.ideal(PolRng, glist)
Ideal generated by
  -5*a1*a2^2*a3 - 2*q*a1^2*a2^2 + 3*a1*a3^3 + a1^3*a3
  -5*a2^3*a3 - 2*q*a1*a2^3 + 3*a2*a3^3 + a1^2*a2*a3
  -2*q*a2^3*a4 + a2*a3^3 + 2*a1*a2*a3*a4 - a1*a3^3

julia> groebas = Oscar.groebner_basis(gidl)  # error occurs here
Gröbner basis with elementsError showing value of type Oscar.IdealGens{AbstractAlgebra.Generic.MPoly{AbstractAlgebra.Generic.RationalFunctionFieldElem{AbsSimpleNumFieldElem, AbstractAlgebra.Generic.Poly{AbsSimpleNumFieldElem}}}}:
ERROR: MethodError: (::AbstractAlgebra.Generic.RationalFunctionField{AbsSimpleNumFieldElem, AbstractAlgebra.Generic.Poly{AbsSimpleNumFieldElem}})(::Singular.n_FieldElem{AbstractAlgebra.Generic.FracFieldElem{AbstractAlgebra.Generic.Poly{AbsSimpleNumFieldElem}}}) is ambiguous.

Candidates:
  (a::AbstractAlgebra.Generic.RationalFunctionField)(b::RingElem)
    @ AbstractAlgebra.Generic ~/.julia/packages/AbstractAlgebra/I018A/src/generic/RationalFunctionField.jl:594
  (b::Ring)(a::Union{Singular.n_FieldElem{T}, Singular.n_RingElem{T}} where T)
    @ Oscar ~/.julia/packages/Oscar/KByfV/src/Rings/mpoly.jl:657

julia is right! This is ambiguous and the fix works only for RationalFunctionField. But the problem persists for any ring type S which has a method of the form (R::S)(x::RingElem).

The problem is that

Oscar.jl/src/Rings/mpoly.jl

Lines 721 to 724 in af90aae

#??? needs to coerce into b? assert parent?
function (b::AbstractAlgebra.Ring)(a::Singular.n_unknown)
Singular.libSingular.julia(Singular.libSingular.cast_number_to_void(a.ptr))::elem_type(b)
end

is too broad in the first argument (or too specific in the second).

I see two options:

  1. Do the dirty for T in subtypes(Ring); @eval trick. Problem is that this is not extensible, so only works for the ring types which have been defined till this point.
  2. Do not use R(x) to do the conversion from the Singular to the Oscar side, but maybe singular_to_oscar(R, x).

Any other ideas? @tthsqe12 @fieker?
@tthsqe12
Copy link
Contributor

method 2. Please relieve us of this R(x) syntax.

@thofma thofma mentioned this issue Feb 6, 2022
Open
@thofma
Copy link
Collaborator Author

thofma commented Feb 7, 2022 •
edited
Loading

As we have all experienced, the conversion between Singular rings and the Oscar side can be a bit error prone. Note that the same problem was exhibited by the GAP <-> Oscar conversions, which was solved by introducing the wonderful function

function iso_oscar_gap(R::Ring)

which returns a map object with proper domain and codomain realising this isomorphism.

I propose that we do the same here and introduce

function iso_oscar_singular(R::MPolyRing)

which returns a map object that one can apply etc.

What do you think @tthsqe12? Since @ederc is also a fan of singular_ring, I am also pinging you here :)

P.S.: We might also need iso_oscar_singular_coefficient_ring, to properly fix the bug in #975.

@thofma thofma mentioned this issue Feb 8, 2022
Closed
@ederc
Copy link
Member

ederc commented Feb 9, 2022

Having discussed this with @thofma I think this might be a good and more secure concept handling conversions.

@thofma thofma mentioned this issue Feb 16, 2022
Closed
@tthsqe12
Copy link
Contributor

tthsqe12 commented Mar 4, 2022

so singular_ring should be no more, and in its place stands
singular_coefficient_ring (number)
singular_polynomial_ring (spoly)
iso_oscar_singular_coefficient_ring (goes both ways)
iso_oscar_singular_polynomial_ring (" ")
?

@thofma
Copy link
Collaborator Author

thofma commented Mar 4, 2022

We probably will remove singular_*_ring altogether and just provide iso_oscar_singular_coefficient_ring/iso_oscar_singular_polynomial_ring.

@tthsqe12
Copy link
Contributor

tthsqe12 commented Mar 4, 2022

ok

@fingolfin
Copy link
Member

What's the status of this? The original issue seems to be resolved, but I see no trace of iso_oscar_singular_* functions. So perhaps there was a different workaround applied? But perhaps we still want iso_oscar_singular_*?

@thofma
Copy link
Collaborator Author

thofma commented Oct 30, 2022 •
edited
Loading

Not resolved. We just fixed the cases that popped up.

@thofma thofma mentioned this issue Apr 18, 2024
Closed
@thofma thofma mentioned this issue Jun 12, 2024
Open
@thofma
Copy link
Collaborator Author

thofma commented Nov 3, 2024

I included a new/still failing in example in the original post.

@thofma thofma linked a pull request Nov 23, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
singular coding sprint triage
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: add iso_oscar_singular_[coeff/poly]_ring oscar-system/Oscar.jl
4 participants
@fingolfin @ederc @tthsqe12 @thofma