search

oscar-system / Singular.jl

Julia package for the Singular library
Other
30 stars 31 forks source link

create correct Singular coeffs with create_ring_from_singular_ring #773

Closed hannes14 closed 5 months ago

hannes14 commented 5 months ago

should help with https://github.com/oscar-system/Oscar.jl/issues/3088

fingolfin commented 5 months ago

Would it be possible to add a minimal test case?

fingolfin commented 5 months ago

ping @hannes14

fingolfin commented 5 months ago

Thanks for adding the test. The OscarCI tests fail with a genuine error:

primary decomposition over AbsNonSimpleNumField: Error During Test at /home/runner/work/Singular.jl/Singular.jl/oscar-dev/Oscar/test/Rings/mpoly.jl:503
  Got exception outside of a @test
  TypeError: in typeassert, expected AbsNonSimpleNumField, got a value of type Singular.N_Field{AbsNonSimpleNumFieldElem}
  Stacktrace:
    [1] base_ring(R::Singular.PolyRing{AbsNonSimpleNumFieldElem})
      @ Singular ~/work/Singular.jl/Singular.jl/src/poly/poly.jl:33
    [2] (::Singular.PolyRing{AbsNonSimpleNumFieldElem})(n::Int64)
      @ Singular ~/work/Singular.jl/Singular.jl/src/poly/poly.jl:1510
    [3] _groebner_basis(r::MPolyQuoRing{AbstractAlgebra.Generic.MPoly{AbsNonSimpleNumFieldElem}})
      @ Oscar ~/work/Singular.jl/Singular.jl/oscar-dev/Oscar/src/Rings/MPolyQuo.jl:37
    [4] singular_origin_groebner_basis(Q::MPolyQuoRing{AbstractAlgebra.Generic.MPoly{AbsNonSimpleNumFieldElem}})
      @ Oscar ~/work/Singular.jl/Singular.jl/oscar-dev/Oscar/src/Rings/MPolyQuo.jl:72
    [5] singular_origin_ring
      @ ~/work/Singular.jl/Singular.jl/oscar-dev/Oscar/src/Rings/MPolyQuo.jl:76 [inlined]
    [6] _simplify(#unused#::Oscar.HasSingularGroebnerAlgorithm, f::MPolyQuoRingElem{AbstractAlgebra.Generic.MPoly{AbsNonSimpleNumFieldElem}})
      @ Oscar ~/work/Singular.jl/Singular.jl/oscar-dev/Oscar/src/Rings/MPolyQuo.jl:863
    [7] simplify
      @ ~/work/Singular.jl/Singular.jl/oscar-dev/Oscar/src/Rings/MPolyQuo.jl:856 [inlined]
    [8] iszero
      @ ~/work/Singular.jl/Singular.jl/oscar-dev/Oscar/src/Rings/MPolyQuo.jl:648 [inlined]
    [9] normalise
      @ ~/.julia/packages/AbstractAlgebra/cwAAG/src/generic/Poly.jl:52 [inlined]
   [10] Poly
      @ ~/.julia/packages/AbstractAlgebra/cwAAG/src/generic/GenericTypes.jl:282 [inlined]
   [11] (::AbstractAlgebra.Generic.PolyRing{MPolyQuoRingElem{AbstractAlgebra.Generic.MPoly{AbsNonSimpleNumFieldElem}}})(b::Vector{MPolyQuoRingElem{AbstractAlgebra.Generic.MPoly{AbsNonSimpleNumFieldElem}}})
      @ AbstractAlgebra.Generic ~/.julia/packages/AbstractAlgebra/cwAAG/src/generic/Poly.jl:372
   [12] gen(R::AbstractAlgebra.Generic.PolyRing{MPolyQuoRingElem{AbstractAlgebra.Generic.MPoly{AbsNonSimpleNumFieldElem}}})
      @ AbstractAlgebra ~/.julia/packages/AbstractAlgebra/cwAAG/src/Poly.jl:201
   [13] #polynomial_ring#126
      @ ~/.julia/packages/AbstractAlgebra/cwAAG/src/NCPoly.jl:741 [inlined]
   [14] _change_poly_ring(R::MPolyQuoRing{AbstractAlgebra.Generic.MPoly{AbsNonSimpleNumFieldElem}}, Rx::AbstractAlgebra.Generic.PolyRing{AbsNonSimpleNumFieldElem}, cached::Bool)
      @ AbstractAlgebra ~/.julia/packages/AbstractAlgebra/cwAAG/src/Poly.jl:3138
   [15] _make_parent(g::Hecke.var"#322#323"{MPolyQuoRing{AbstractAlgebra.Generic.MPoly{AbsNonSimpleNumFieldElem}}, Hecke.MapDataFromNfAbsNS{Vector{MPolyQuoRingElem{AbstractAlgebra.Generic.MPoly{AbsNonSimpleNumFieldElem}}}}}, p::AbstractAlgebra.Generic.Poly{AbsNonSimpleNumFieldElem}, cached::Bool)
      @ AbstractAlgebra ~/.julia/packages/AbstractAlgebra/cwAAG/src/Poly.jl:3176
   [16] map_data_given_base_field_data(K::Hecke.RelSimpleNumField{AbsNonSimpleNumFieldElem}, L::MPolyQuoRing{AbstractAlgebra.Generic.MPoly{AbsNonSimpleNumFieldElem}}, z::Hecke.MapDataFromNfAbsNS{Vector{MPolyQuoRingElem{AbstractAlgebra.Generic.MPoly{AbsNonSimpleNumFieldElem}}}}, y::MPolyQuoRingElem{AbstractAlgebra.Generic.MPoly{AbsNonSimpleNumFieldElem}}; check::Bool)
      @ Hecke ~/.julia/packages/Hecke/wehok/src/Map/NumField.jl:364
   [17] #map_data#325
      @ ~/.julia/packages/Hecke/wehok/src/Map/NumField.jl:399 [inlined]
   [18] #hom#316
      @ ~/.julia/packages/Hecke/wehok/src/Map/NumField.jl:151 [inlined]
   [19] hom
      @ ~/.julia/packages/Hecke/wehok/src/Map/NumField.jl:142 [inlined]
   [20] _expand_coefficient_field(R::AbstractAlgebra.Generic.MPolyRing{Hecke.RelSimpleNumFieldElem{AbsNonSimpleNumFieldElem}}; rec_depth::Int64)
      @ Oscar ~/work/Singular.jl/Singular.jl/oscar-dev/Oscar/src/Rings/primary_decomposition_helpers.jl:38
   [21] #_expand_coefficient_field_to_QQ#1182
      @ ~/work/Singular.jl/Singular.jl/oscar-dev/Oscar/src/Rings/primary_decomposition_helpers.jl:114 [inlined]
   [22] _expand_coefficient_field_to_QQ
      @ ~/work/Singular.jl/Singular.jl/oscar-dev/Oscar/src/Rings/primary_decomposition_helpers.jl:114 [inlined]
   [23] primary_decomposition(I::MPolyIdeal{AbstractAlgebra.Generic.MPoly{Hecke.RelSimpleNumFieldElem{AbsNonSimpleNumFieldElem}}}; algorithm::Symbol, cache::Bool)
      @ Oscar ~/work/Singular.jl/Singular.jl/oscar-dev/Oscar/src/Rings/mpoly-ideals.jl:725
   [24] primary_decomposition(I::MPolyIdeal{AbstractAlgebra.Generic.MPoly{Hecke.RelSimpleNumFieldElem{AbsNonSimpleNumFieldElem}}})
      @ Oscar ~/work/Singular.jl/Singular.jl/oscar-dev/Oscar/src/Rings/mpoly-ideals.jl:721
   [25] macro expansion
      @ ~/work/Singular.jl/Singular.jl/oscar-dev/Oscar/test/Rings/mpoly.jl:510 [inlined]
   [26] macro expansion
      @ /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.6/Test/src/Test.jl:1151 [inlined]
   [27] top-level scope
      @ ~/work/Singular.jl/Singular.jl/oscar-dev/Oscar/test/Rings/mpoly.jl:504
   [28] include
      @ ./Base.jl:385 [inlined]
   [29] macro expansion
      @ ./timing.jl:368 [inlined]
   [30] _timed_include(str::String, mod::Module; use_ctime::Bool)
      @ Oscar ~/work/Singular.jl/Singular.jl/oscar-dev/Oscar/src/utils/tests.jl:5
   [31] (::Oscar.var"#34#35"{Bool, Bool, Bool, Vector{Any}})()
      @ Oscar ~/work/Singular.jl/Singular.jl/oscar-dev/Oscar/src/utils/tests.jl:175
   [32] with_unicode(f::Oscar.var"#34#35"{Bool, Bool, Bool, Vector{Any}}, allowed::Bool)
      @ AbstractAlgebra.PrettyPrinting ~/.julia/packages/AbstractAlgebra/cwAAG/src/PrettyPrinting.jl:1707
   [33] #test_module#33
      @ ~/work/Singular.jl/Singular.jl/oscar-dev/Oscar/src/utils/tests.jl:123 [inlined]
   [34] #29
      @ ~/work/Singular.jl/Singular.jl/oscar-dev/Oscar/test/runtests.jl:137 [inlined]
   [35] (::Base.var"#834#839"{var"#29#30"})(r::Base.RefValue{Any}, args::Tuple{String})
      @ Base ./asyncmap.jl:100
   [36] macro expansion
      @ ./asyncmap.jl:234 [inlined]
   [37] (::Base.var"#850#851"{Base.var"#834#839"{var"#29#30"}, Channel{Any}, Nothing})()
      @ Base ./task.jl:417
Test Summary:                                   | Error  Total

@hannes14 could you have a look what's going on there?

fingolfin commented 5 months ago

OK the error can be reproduce here, too, if one changes the final line of the new test to

@test base_ring(R) == base_ring(S)

which is arguably what we should use anyway.

fingolfin commented 5 months ago

I've pushed a change which should fix the immediate issue, I think, but let's see what CI reports.

fingolfin commented 5 months ago

I have another potential simplification of the code: use basering = CoefficientRing(R) ! (which also deals with rings whose type is immutable). But I'd rather first let the CI of this version run to completion before trying yet another variant.

hannes14 commented 5 months ago

@fingolfin Passes the tests,shall be merged.

fingolfin commented 5 months ago

Merged. Now libsingular_jll needs to be updated again, and after Singular.jl, and then a release... Can you take care of it, @hannes14 ?