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

creating rings from the rings that singular creates #336

Merged
merged 2 commits into from
Feb 3, 2021

Conversation

tthsqe12
Copy link
Contributor

@tthsqe12 tthsqe12 commented Jan 5, 2021

This is not serious yet and cannot be tried because it requires modifications to the jll's. I am just looking for comments and suggestions at this point. The main problem is creating a meaningful coefficient ring from the ring that singular returns. Since there is a wide variety of possibilities here, it seems impossible to cover all cases. Therefore, I propose having an UnknownSingularCoefficientRing with elements of type n_unknownsingularcoefficient as a fallback and implementing the search for special cases as we need them. For example, in #187 the coefficient ring does not have a meaningful Nemo equivalent. Here is what it currently looks like (BTW I find it ironic that the answer is called SOL:):

julia> R, (s,) = PolynomialRing(Singular.QQ, ["s"])
(Singular Polynomial Ring (QQ),(s),(dp(1),C), spoly{n_Q}[s])

julia> I = Singular.Ideal(R, [s^33-3*s^32-6*s^31-s^30+3*s^23-s^15])
Singular Ideal over Singular Polynomial Ring (QQ),(s),(dp(1),C) with generators (s^33-3*s^32-6*s^31-s^30+3*s^23-s^15)

julia> Singular.LibSolve.solve(I)
...
// 'solve' created a ring, in which a list SOL of numbers (the complex solutions)
// is stored.
// To access the list of complex solutions, type (if the name R was assigned
// to the return value):
        setring R; SOL; 
2-element Array{Any,1}:
 Singular Polynomial Ring (complex,8,8,i),(s),(lp(1),C)
 Dict{Symbol,Any}(:SOL => Singular.n_unknownsingularcoefficient[-0.89521895, 4.4114706, (-1.1482541+i*0.17026655), (-1.1482541-i*0.17026655), (-0.66557296+i*0.58989253), (-0.66557296-i*0.58989253), (-0.56567524+i*0.74847289), (-0.56567524-i*0.74847289), (-0.090798442+i*0.87424398), (-0.090798442-i*0.87424398), (0.11962281+i*0.88093791), (0.11962281-i*0.88093791), (0.53509334+i*0.68993065), (0.53509334-i*0.68993065), (0.69319204+i*0.53157745), (0.69319204-i*0.53157745), (0.86426672-i*0.11380014), (0.86426672+i*0.11380014), 0])

julia> ans[2][:SOL]
19-element Array{Singular.n_unknownsingularcoefficient,1}:
 -0.89521895
 4.4114706
 (-1.1482541+i*0.17026655)
 (-1.1482541-i*0.17026655)
 (-0.66557296+i*0.58989253)
 (-0.66557296-i*0.58989253)
 (-0.56567524+i*0.74847289)
 (-0.56567524-i*0.74847289)
 (-0.090798442+i*0.87424398)
 (-0.090798442-i*0.87424398)
 (0.11962281+i*0.88093791)
 (0.11962281-i*0.88093791)
 (0.53509334+i*0.68993065)
 (0.53509334-i*0.68993065)
 (0.69319204+i*0.53157745)
 (0.69319204-i*0.53157745)
 (0.86426672-i*0.11380014)
 (0.86426672+i*0.11380014)
 0

julia> ans[10]
(-0.090798442-i*0.87424398)

julia> typeof(ans)
Singular.n_unknownsingularcoefficient

@thofma
Copy link
Collaborator

thofma commented Jan 5, 2021

Can one do anything useful with those objects except for letting Singular print them?

I am not sure if (as a user) I would rather see an error that something is not implemented yet or get something useless as a return value.

@tthsqe12
Copy link
Contributor Author

tthsqe12 commented Jan 5, 2021

You can do useful stuff with them, but let me first try to explain the situation as I see it. The singular coefficient arithmetic is implement via table lookup. So, even thought we have all of these coefficient types, what they really are doing is just setting up a lookup table: look at the implementation of + for n_GF and n_Q - they are exactly the same. Therefore, the separate coefficient types T, the parametrization of the poly rings by T, and the parametrization of the polynomial types by T seems a bit suspicious to me. It seems to me that this parametrization is currently gaining us nothing: the code for + on spoly does not depend on T but multiple copies of it will be compiled for each T (right?), and I do not see how the compiler can deduce the return type of the coefficient iterator in poly.jl.
All of the stuff for n_unknownsingularcoefficient would be implemented the same way, and stuff that can't be done without singular can't be done.

@thofma
Copy link
Collaborator

thofma commented Jan 6, 2021

I do not see how the compiler can deduce the return type of the coefficient iterator in poly.jl.

The pointer is passed through the base ring, which makes it inferable for julia.

I think one reason to tag T onto everything is to make the information about the type of the coefficient available in the type domain on the julia side. To play nice with the rest with Oscar, we need to know, for example, if a polynomial/matrix is a polynomial/matrix over a field or a ring. If everything were just pointers and we would have to ask for isfield(x::GenericSingularElement), this would not be possible (easily).

Maybe the implementation could be consolidated/refactored to generate it using some metaprogramming and for T in [n_Zn, n_Qn, n_unkownsingularcoefficient; @eval ...`.

@tthsqe12
Copy link
Contributor Author

tthsqe12 commented Jan 6, 2021

The pointer is passed through the base ring, which makes it inferable for julia.

I do not see how this is possible. Maybe you could explain?
The type is here https://github.com/oscar-system/Singular.jl/blob/master/src/poly/PolyTypes.jl#L10
The iterator return is here https://github.com/oscar-system/Singular.jl/blob/master/src/poly/poly.jl#L265
I simply do not see how T can determine the return type of R(...). The problem is that the base_ring member of PolyRing{T} could be any ring and it knows nothing about T.

I am not against parametric poly types for singular.jl, I am just curious about this implementation.

Anyways, the possible singular coefficient rings are here https://www.singular.uni-kl.de/Manual/4-0-3/sing_28.htm.
Of course this is not complete anymore because generic nemo coefficient rings are possible too. I have no idea how these interact with
3. a localization of 1.
4. a quotient ring by an ideal of 1. or 2.,
5. a tensor product of 1. or 2.

As for the actual coefficient rings, the ones missing from singular.jl are
5. simple algebraic extension of $Q$ or $Z/p$,
6. the field of real numbers represented by floating point numbers of a user defined precision,
7. the field of complex numbers represented by (pairs of) floating point numbers of a user defined precision,

Is it desired here to implement superficial wrapper types for each of these? They would be superficial because at the end of the day, the addition is implemented via

function +(x, y)
   c = parent(x)
   p = libSingular.n_Add(x.ptr, y.ptr, c.ptr)
   return c(p)
end

@thofma
Copy link
Collaborator

thofma commented Jan 6, 2021

The pointer is passed through the base ring, which makes it inferable for julia.

I do not see how this is possible. Maybe you could explain?
The type is here https://github.com/oscar-system/Singular.jl/blob/master/src/poly/PolyTypes.jl#L10
The iterator return is here https://github.com/oscar-system/Singular.jl/blob/master/src/poly/poly.jl#L265
I simply do not see how T can determine the return type of R(...). The problem is that the base_ring member of PolyRing{T} could be any ring and it knows nothing about T.

I am not against parametric poly types for singular.jl, I am just curious about this implementation.

Oh, right. We are just missing the type annotation base_ring(R::PolyRing{T}) = R.base_ring::parent_type(T) at

base_ring(R::PolyRing{T}) where T <: Nemo.RingElem = R.base_ring
and then everything is fine and inferable. So there is nothing bad for the compiler going on here.

Do you have an alternative to the superficial wrapper approach so that we can use AbstractAlgebra with Singular types?

@tthsqe12
Copy link
Contributor Author

tthsqe12 commented Jan 13, 2021

This requires oscar-system/libsingular-julia#9
The test code contains the new stuff that works.

@tthsqe12 tthsqe12 marked this pull request as ready for review January 13, 2021 15:32
@thofma
Copy link
Collaborator

thofma commented Jan 15, 2021

I updated the binaries. Seems to work.

@tthsqe12
Copy link
Contributor Author

tthsqe12 commented Jan 15, 2021

Thanks. For those interested: the coefficient rings go through the proper cache mechanism, but the created polynomial rings do not because I have to create rings with ordering :unknown which does not uniquely identify the ordering.

function create_ring_from_singular_ring(r::libSingular.ring_ptr)
ordering = libSingular.ring_ordering_as_symbol(r)
c = libSingular.rCoeffPtr(r)
if libSingular.nCoeff_is_Q(c)
Copy link
Member

Choose a reason for hiding this comment

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

I suggest we also wrap getCoeffType, which is also useful for interactive debugging

@fingolfin
Copy link
Member

What is the status of this?

#
###############################################################################

mutable struct N_UnknownSingularCoefficientRing <: Ring
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this duplicate the functionality of struct CoefficientRing together with struct n_unknown, defined just above? Or if not, what's the difference? (In the latter case, a comment should explain it?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The CoefficientRing{T} type has a field base_ring::Nemo.Ring which we don't always have. This new type is really for the coefficient rings that singular creates that we cannot identify/there is no meaningful equivalent in julia. For example, singular has floating point coefficients, and I am not really keen on creating a special floating point coefficient type for singular.jl.

The comment is a few lines up:

#   N_UnknownSingularCoefficientRing
#   singular coefficient rings with no identifiable julia object

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know this new unknown coefficient type is not elegant, but I don't see another way if we are not able to identify what the coefficient ring is. It is meant as a default case in this switch:

@tthsqe12
Copy link
Contributor Author

The status is: I am happy with it for now. The caching/uniqueness issue will probably be important later, and I have no idea how to solve it currently.

@fieker
Copy link
Contributor

fieker commented Feb 3, 2021

why is this not merged if everyone is happy? Any particular reason?

@tthsqe12 tthsqe12 merged commit f3d27df into oscar-system:master Feb 3, 2021
fingolfin pushed a commit to fingolfin/Singular.jl that referenced this pull request Jun 6, 2023
creating rings from the rings that singular creates
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.

4 participants