-
Notifications
You must be signed in to change notification settings - Fork 68
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
change_base_ring desired behaviour #490
Comments
Can we not do that? It's not the "base ring" functionality from
I guess you meant
So, to take the example from the issue, does it mean that |
I thought similar was now supposed to create an uninitialised matrix. Why should it care what the base ring is? The only reason to pass that in is if it were going to create an initialised object. It should only need the type of elements in the base ring, not the actual base ring itself. If that's not actually possible, then we should not be using similar and should go back to how things were, i.e. requiring a AbstractAlgebra/Nemo matrix to always be constructed in an initialised state. The fact that we could get around the problem by forcing the user to supply a base ring, does not mean we should. It's inconsistent semantics. |
That was the idea indeed, while
Because for example, AA matrices contain a field That said, I don't see a big problem with also having
It's possible, but I don't understand you conclusion that all matrices always have to be constructed in an initialised state. It's good to usually return a matrix in an initialized state of course, but a low level tool to return an unitialized matrix has it place, for when you implement a routine which initializes all the entries of the matrix anyway. Again,
Sorry, I don't understand this sentence, I don't remember where we forced the user to supply a base ring (in |
I've now updated the steps above after a long discussion with @rfourquet The similar/map_coeffs/map_entries functions will now be implemented using a different strategy and the change_base_ring functionality will not (cannot) depend on it. So the two issues are now orthogonal. |
I believe we now have change_base_ring sorted out (though perhaps not the way we thought). So I will close this issue. |
@thofma and I had a (very) long discussion about what behaviour we ultimately want.
In his case, he wants change_base_ring(ZZ, M) in Nemo, where M is a Generic.MatSpaceElem{nf_elem} to return an fmpz_mat. Reason: both fmpz_mat and Generic.MatSpaceElem{nf_elem} are matrices that Nemo would create.
In my case, I want change_base_ring(Nemo.ZZ, p) in Singular, where p is a Singular spoly{fmpq} to return an spoly{fmpz}. Reason: both polynomials are what Singular would create over a Nemo coefficient ring.
So we don't disagree on what we want, it's just not implemented that way at present (and probably wasn't implemented correctly before the new similar functionality, either).
The following simple 7 steps will lead to balance being restored in the force:
[Removed] see the issue for similar/map which is now essentially orthogonal to the change_base_ring issue.
Create abstract types NemoMatrixElem{T} <: AbstractAlgebra.MatrixElem{T}, NemoPolyElem{T} <: AbstractAlgebra.PolyElem{T}, NemoMPolyElem{T} <: AbstractAlgebra.MPolyElem{T} and make all Nemo matrix and polynomial element types belong to one of these, instead of directly to the AbstractAlgebra abstract types.
2a. Create an abstract type NemoRing <: AbstractAlgebra.Ring and make all Nemo ring element types belong to this instead of directly to the AbstractAlgebra abstract type.
In AbstractAlgebra, implement generic fallback change_base_ring functions, which always creates generic Mat/Poly/MPoly's. It will have signature, e.g: change_base_ring(::Ring, ::AbstractAlgebra.MatElem).
For Nemo rings with native matrix, poly or mpoly types, e.g. fmpz, implement change_base_ring(::fmpz, ::NemoMat). This should always create a Nemo matrix (fmpz_mat, in the example).
For Nemo rings without native matrix, poly or mpoly types, e.g. nf_elem, implement change_base_ring(::NemoRing, Generic.MatSpaceElem{nf_elem}) which will always construct a Nemo matrix. This will ensure @thofma gets the functionality he wants.
change_base_ring will NOT be implemented in terms of similar.
[Removed]
I don't discuss here how we handle the Singular functionality, which allows for creating Singular polynomials over Nemo coefficient rings and vice versa. But it is clear that the strategy above is consistent across all our systems and will work there too.
The text was updated successfully, but these errors were encountered: