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

Ensure MatElem subtypes T have T(R::Ring, r::Int, c::Int) constructor #1791

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/arb/ArbTypes.jl
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,9 @@
rows::Ptr{Nothing}
#base_ring::ArbField

# MatElem interface
RealMat(::RealField, r::Int, c::Int) = RealMat(r, c)

Check warning on line 800 in src/arb/ArbTypes.jl

View check run for this annotation

Codecov / codecov/patch

src/arb/ArbTypes.jl#L800

Added line #L800 was not covered by tests

function RealMat(r::Int, c::Int)
z = new()
ccall((:arb_mat_init, libflint), Nothing, (Ref{RealMat}, Int, Int), z, r, c)
Expand Down Expand Up @@ -888,6 +891,13 @@
rows::Ptr{Nothing}
base_ring::ArbField

# MatElem interface
function ArbMatrix(R::ArbField, r::Int, c::Int)
z = ArbMatrix(r, c)
z.base_ring = R
return z
end

function ArbMatrix(r::Int, c::Int)
z = new()
ccall((:arb_mat_init, libflint), Nothing, (Ref{ArbMatrix}, Int, Int), z, r, c)
Expand Down Expand Up @@ -984,6 +994,9 @@
rows::Ptr{Nothing}
#base_ring::AcbField

# MatElem interface
ComplexMat(::ComplexField, r::Int, c::Int) = ComplexMat(r, c)

Check warning on line 998 in src/arb/ArbTypes.jl

View check run for this annotation

Codecov / codecov/patch

src/arb/ArbTypes.jl#L998

Added line #L998 was not covered by tests

function ComplexMat(r::Int, c::Int)
z = new()
ccall((:acb_mat_init, libflint), Nothing, (Ref{ComplexMat}, Int, Int), z, r, c)
Expand Down Expand Up @@ -1186,6 +1199,13 @@
rows::Ptr{Nothing}
base_ring::AcbField

# MatElem interface
function AcbMatrix(R::AcbField, r::Int, c::Int)
z = AcbMatrix(r, c)
z.base_ring = R
return z
end

function AcbMatrix(r::Int, c::Int)
z = new()
ccall((:acb_mat_init, libflint), Nothing, (Ref{AcbMatrix}, Int, Int), z, r, c)
Expand Down
12 changes: 4 additions & 8 deletions src/arb/acb_mat.jl
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@
###############################################################################

function similar(::AcbMatrix, R::AcbField, r::Int, c::Int)
Copy link
Member Author

Choose a reason for hiding this comment

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

In the future this and its many siblings could be replaced by a single default method

similar(::T, R::AcbField, r::Int, c::Int) where T <: MatElem = T(R, r, c)

z = AcbMatrix(r, c)
z.base_ring = R
z = AcbMatrix(R, r, c)
return z
end

Expand Down Expand Up @@ -747,8 +746,7 @@ end
###############################################################################

function (x::AcbMatSpace)()
Copy link
Member Author

Choose a reason for hiding this comment

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

Currently we already have a default method for this which is

function (a::MatSpace{T})() where {T <: NCRingElement}
   return zero_matrix(base_ring(a), nrows(a), ncols(a))::dense_matrix_type(T)
end

but that means we implicitly expect zero_matrix to be implemented... With the new system both can be implemented generically.

z = AcbMatrix(nrows(x), ncols(x))
z.base_ring = x.base_ring
z = AcbMatrix(base_ring(x), nrows(x), ncols(x))
return z
end

Expand Down Expand Up @@ -893,8 +891,7 @@ function zero_matrix(R::AcbField, r::Int, c::Int)
if r < 0 || c < 0
error("dimensions must not be negative")
end
z = AcbMatrix(r, c)
z.base_ring = R
z = AcbMatrix(R, r, c)
return z
end

Expand All @@ -908,9 +905,8 @@ function identity_matrix(R::AcbField, n::Int)
if n < 0
error("dimension must not be negative")
end
z = AcbMatrix(n, n)
z = AcbMatrix(R, n, n)
ccall((:acb_mat_one, libflint), Nothing, (Ref{AcbMatrix}, ), z)
z.base_ring = R
return z
end

Expand Down
15 changes: 5 additions & 10 deletions src/arb/arb_mat.jl
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@
###############################################################################

function similar(::ArbMatrix, R::ArbField, r::Int, c::Int)
z = ArbMatrix(r, c)
z.base_ring = R
z = ArbMatrix(R, r, c)
return z
end

Expand Down Expand Up @@ -84,9 +83,8 @@ number_of_rows(a::ArbMatrix) = a.r
number_of_columns(a::ArbMatrix) = a.c

function deepcopy_internal(x::ArbMatrix, dict::IdDict)
z = ArbMatrix(nrows(x), ncols(x))
z = ArbMatrix(base_ring(x), nrows(x), ncols(x))
ccall((:arb_mat_set, libflint), Nothing, (Ref{ArbMatrix}, Ref{ArbMatrix}), z, x)
z.base_ring = x.base_ring
return z
end

Expand Down Expand Up @@ -713,8 +711,7 @@ end
###############################################################################

function (x::ArbMatSpace)()
z = ArbMatrix(nrows(x), ncols(x))
z.base_ring = x.base_ring
z = ArbMatrix(base_ring(x), nrows(x), ncols(x))
return z
end

Expand Down Expand Up @@ -789,8 +786,7 @@ function zero_matrix(R::ArbField, r::Int, c::Int)
if r < 0 || c < 0
error("dimensions must not be negative")
end
z = ArbMatrix(r, c)
z.base_ring = R
z = ArbMatrix(R, r, c)
return z
end

Expand All @@ -804,9 +800,8 @@ function identity_matrix(R::ArbField, n::Int)
if n < 0
error("dimension must not be negative")
end
z = ArbMatrix(n, n)
z = ArbMatrix(R, n, n)
ccall((:arb_mat_one, libflint), Nothing, (Ref{ArbMatrix}, ), z)
z.base_ring = R
return z
end

Expand Down
69 changes: 57 additions & 12 deletions src/flint/FlintTypes.jl
Original file line number Diff line number Diff line change
Expand Up @@ -4738,7 +4738,10 @@
rows::Ptr{Nothing}
view_parent

# used by windows, not finalised!!
# MatElem interface
QQMatrix(::QQField, r::Int, c::Int) = QQMatrix(r, c)

Check warning on line 4742 in src/flint/FlintTypes.jl

View check run for this annotation

Codecov / codecov/patch

src/flint/FlintTypes.jl#L4742

Added line #L4742 was not covered by tests

# Used by view, not finalised!!
function QQMatrix()
return new()
end
Expand Down Expand Up @@ -4860,6 +4863,9 @@
rows::Ptr{Nothing}
view_parent

# MatElem interface
ZZMatrix(::ZZRing, r::Int, c::Int) = ZZMatrix(r, c)

Check warning on line 4867 in src/flint/FlintTypes.jl

View check run for this annotation

Codecov / codecov/patch

src/flint/FlintTypes.jl#L4867

Added line #L4867 was not covered by tests

# Used by view, not finalised!!
function ZZMatrix()
return new()
Expand Down Expand Up @@ -4958,10 +4964,16 @@
base_ring::zzModRing
view_parent

# MatElem interface
function zzModMatrix(R::zzModRing, r::Int, c::Int)
z = zzModMatrix(r, c, R.n)
z.base_ring = R
return z

Check warning on line 4971 in src/flint/FlintTypes.jl

View check run for this annotation

Codecov / codecov/patch

src/flint/FlintTypes.jl#L4968-L4971

Added lines #L4968 - L4971 were not covered by tests
end

# Used by view, not finalised!!
function zzModMatrix()
z = new()
return z
return new()
end

function zzModMatrix(r::Int, c::Int, n::UInt)
Expand Down Expand Up @@ -5102,10 +5114,16 @@
base_ring::ZZModRing
view_parent

# MatElem interface
function ZZModMatrix(R::ZZModRing, r::Int, c::Int)
z = ZZModMatrix(r, c, R.ninv)
z.base_ring = R
return z
end

# Used by view, not finalised!!
function ZZModMatrix()
z = new()
return z
return new()
end

function ZZModMatrix(r::Int, c::Int, ctx::fmpz_mod_ctx_struct)
Expand Down Expand Up @@ -5260,10 +5278,16 @@
base_ring::FpField
view_parent

# MatElem interface
function FpMatrix(R::FpField, r::Int, c::Int)
z = FpMatrix(r, c, R.ninv)
z.base_ring = R
return z
end

# Used by view, not finalised!!
function FpMatrix()
z = new()
return z
return new()
end

function FpMatrix(r::Int, c::Int, ctx::fmpz_mod_ctx_struct)
Expand Down Expand Up @@ -5370,10 +5394,16 @@
base_ring::fpField
view_parent

# MatElem interface
function fpMatrix(R::fpField, r::Int, c::Int)
z = fpMatrix(r, c, R.n)
z.base_ring = R
return z

Check warning on line 5401 in src/flint/FlintTypes.jl

View check run for this annotation

Codecov / codecov/patch

src/flint/FlintTypes.jl#L5398-L5401

Added lines #L5398 - L5401 were not covered by tests
end

# Used by view, not finalised!!
function fpMatrix()
z = new()
return z
return new()
end

function fpMatrix(r::Int, c::Int, n::UInt)
Expand Down Expand Up @@ -5948,7 +5978,12 @@
base_ring::FqField
view_parent

# used by windows, not finalised!!
# MatElem interface
function FqMatrix(R::FqField, r::Int, c::Int)
return FqMatrix(r, c, R)

Check warning on line 5983 in src/flint/FlintTypes.jl

View check run for this annotation

Codecov / codecov/patch

src/flint/FlintTypes.jl#L5982-L5983

Added lines #L5982 - L5983 were not covered by tests
Copy link
Member Author

Choose a reason for hiding this comment

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

For FqMatrix we already have FqMatrix(r, c, R) but I don't see a downside with having both that and FqMatrix(R, r, c). Down the road we could think about abolishing the former in a breaking release, but there is no strong need either.

(One could also argue that we should just use the (r, c, R) order everywhere. Would be also OK to me. I used the former because (a) the old PR used it and (b) it matches the order in matrix(R, r, c, ...), zero_matrix, and some other places

I think @thofma added the FqMatrix constructor here, so perhaps he had a deeper reason for the different order that we should heed?

Copy link
Member

Choose a reason for hiding this comment

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

The reason is that I copy & pasted it from some other matrix constructor in Nemo. There is no deeper reason. It is not part of the API, so can be changed/augmented/removed.

end

# Used by view, not finalised!!
function FqMatrix()
return new()
end
Expand Down Expand Up @@ -6114,7 +6149,12 @@
base_ring::FqPolyRepField
view_parent

# used by windows, not finalised!!
# MatElem interface
function FqPolyRepMatrix(R::FqPolyRepField, r::Int, c::Int)
return FqPolyRepMatrix(r, c, R)

Check warning on line 6154 in src/flint/FlintTypes.jl

View check run for this annotation

Codecov / codecov/patch

src/flint/FlintTypes.jl#L6153-L6154

Added lines #L6153 - L6154 were not covered by tests
end

# Used by view, not finalised!!
function FqPolyRepMatrix()
return new()
end
Expand Down Expand Up @@ -6247,7 +6287,12 @@
base_ring::fqPolyRepField
view_parent

# used by windows, not finalised!!
# MatElem interface
function fqPolyRepMatrix(R::fqPolyRepField, r::Int, c::Int)
return fqPolyRepMatrix(r, c, R)

Check warning on line 6292 in src/flint/FlintTypes.jl

View check run for this annotation

Codecov / codecov/patch

src/flint/FlintTypes.jl#L6291-L6292

Added lines #L6291 - L6292 were not covered by tests
end

# Used by view, not finalised!!
function fqPolyRepMatrix()
return new()
end
Expand Down
12 changes: 4 additions & 8 deletions src/flint/fmpz_mod_mat.jl
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ dense_matrix_type(::Type{ZZModRingElem}) = ZZModMatrix
###############################################################################

function similar(::MatElem, R::ZZModRing, r::Int, c::Int)
z = ZZModMatrix(r, c, R.ninv)
z.base_ring = R
z = ZZModMatrix(R, r, c)
return z
end

Expand Down Expand Up @@ -749,8 +748,7 @@ promote_rule(::Type{ZZModMatrix}, ::Type{ZZRingElem}) = ZZModMatrix
################################################################################

function (a::ZZModMatrixSpace)()
z = ZZModMatrix(nrows(a), ncols(a), base_ring(a).ninv)
z.base_ring = a.base_ring
z = ZZModMatrix(base_ring(a), nrows(a), ncols(a))
return z
end

Expand Down Expand Up @@ -814,11 +812,10 @@ end

function (a::ZZModMatrixSpace)(b::ZZMatrix)
(ncols(a) != b.c || nrows(a) != b.r) && error("Dimensions do not fit")
z = ZZModMatrix(b.r, b.c, base_ring(a).ninv)
z = ZZModMatrix(base_ring(a), b.r, b.c)
ccall((:fmpz_mod_mat_set_fmpz_mat, libflint), Nothing,
(Ref{ZZModMatrix}, Ref{ZZMatrix}, Ref{fmpz_mod_ctx_struct}),
z, b, base_ring(a).ninv)
z.base_ring = a.base_ring
return z
end

Expand Down Expand Up @@ -851,8 +848,7 @@ function zero_matrix(R::ZZModRing, r::Int, c::Int)
if r < 0 || c < 0
error("dimensions must not be negative")
end
z = ZZModMatrix(r, c, R.ninv)
z.base_ring = R
z = ZZModMatrix(R, r, c)
return z
end

Expand Down
9 changes: 3 additions & 6 deletions src/flint/gfp_fmpz_mat.jl
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ dense_matrix_type(::Type{FpFieldElem}) = FpMatrix
###############################################################################

function similar(::MatElem, R::FpField, r::Int, c::Int)
z = FpMatrix(r, c, R.ninv)
z.base_ring = R
z = FpMatrix(R, r, c)
return z
end

Expand Down Expand Up @@ -261,8 +260,7 @@ end
################################################################################

function (a::FpMatrixSpace)()
z = FpMatrix(nrows(a), ncols(a), base_ring(a).ninv)
z.base_ring = a.base_ring
z = FpMatrix(base_ring(a), nrows(a), ncols(a))
return z
end

Expand Down Expand Up @@ -362,8 +360,7 @@ function zero_matrix(R::FpField, r::Int, c::Int)
if r < 0 || c < 0
error("dimensions must not be negative")
end
z = FpMatrix(r, c, R.ninv)
z.base_ring = R
z = FpMatrix(R, r, c)
return z
end

Expand Down
Loading