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

Remove duplicate *cat implementations #1467

Merged
merged 2 commits into from
Nov 21, 2023
Merged

Remove duplicate *cat implementations #1467

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
09a9541
Remove duplicate `*cat` implementations
lgoettgens Oct 13, 2023
5a51616
Add comments to bad reduce dispatches
lgoettgens Oct 13, 2023
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
2 changes: 2 additions & 0 deletions src/Matrix.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6496,6 +6496,7 @@
return _vcat(A)
end

# this leads to an ambiguity when calling `reduce(hcat, Union{}[])`, but we don't have a better solution right now

Check warning on line 6499 in src/Matrix.jl

View check run for this annotation

Codecov / codecov/patch

src/Matrix.jl#L6499 

Added line #L6499 was not covered by tests
Base.reduce(::typeof(vcat), A::AbstractVector{<:MatrixElem}) = _vcat(A)

function _vcat(A)
Expand Down Expand Up @@ -6534,6 +6535,7 @@
return _hcat(A)
end

# this leads to an ambiguity when calling `reduce(hcat, Union{}[])`, but we don't have a better solution right now
Base.reduce(::typeof(hcat), A::AbstractVector{<:MatrixElem}) = _hcat(A)

function _hcat(A)
Expand Down
123 changes: 0 additions & 123 deletions src/NemoStuff.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,129 +176,6 @@ function sub(M::Generic.Mat, rows::AbstractUnitRange{Int}, cols::AbstractUnitRan
return z
end

################################################################################
#
# Concatenation of matrices
#
################################################################################

@doc raw"""
vcat(A::Vector{Mat}) -> Mat

Forms a big matrix by vertically concatenating the matrices in $A$.
All component matrices need to have the same number of columns.
"""
function Base.vcat(A::Vector{T}) where {S<:RingElem,T<:MatElem{S}}
if any(x -> ncols(x) != ncols(A[1]), A)
error("Matrices must have same number of columns")
end
M = zero_matrix(base_ring(A[1]), sum(nrows, A), ncols(A[1]))
s = 0
for i = A
for j = 1:nrows(i)
for k = 1:ncols(i)
M[s+j, k] = i[j, k]
end
end
s += nrows(i)
end
return M
end

function Base.hcat(A::Vector{T}) where {S<:RingElem,T<:MatElem{S}}
if any(x -> nrows(x) != nrows(A[1]), A)
error("Matrices must have same number of rows")
end
M = zero_matrix(base_ring(A[1]), nrows(A[1]), sum(ncols, A))
s = 0
for i = A
for j = 1:ncols(i)
for k = 1:nrows(i)
M[k, s+j] = i[k, j]
end
end
s += ncols(i)
end
return M
end

function Base.hcat(A::MatElem...)
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need to remove this? I just tried with removing just the preceding methods, and vcat(Union{}[]) as well as vcat() worked just fine.

Same question for the other methods taking ::MatElem... args

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They already exist in better in src/Matrix.jl. So these removed methods here get never actually called

r = nrows(A[1])
c = ncols(A[1])
R = base_ring(A[1])
for i = 2:length(A)
@assert nrows(A[i]) == r
@assert base_ring(A[i]) == R
c += ncols(A[i])
end
X = zero_matrix(R, r, c)
o = 1
for i = 1:length(A)
for j = 1:ncols(A[i])
X[:, o] = A[i][:, j]
o += 1
end
end
return X
end

function Base.cat(A::MatElem...; dims)
@assert dims == (1, 2) || isa(dims, Int)

if isa(dims, Int)
if dims == 1
return hcat(A...)
elseif dims == 2
return vcat(A...)
else
error("dims must be 1, 2, or (1,2)")
end
end

local X
for i = 1:length(A)
if i == 1
X = hcat(A[1], zero_matrix(base_ring(A[1]), nrows(A[1]), sum(Int[ncols(A[j]) for j = 2:length(A)])))
else
X = vcat(X, hcat(zero_matrix(base_ring(A[1]), nrows(A[i]), sum(ncols(A[j]) for j = 1:i-1)), A[i], zero_matrix(base_ring(A[1]), nrows(A[i]), sum(Int[ncols(A[j]) for j = i+1:length(A)]))))
end
end
return X
end

#= seems to be in AA now
function Base.hvcat(rows::Tuple{Vararg{Int}}, A::MatElem...)
B = hcat([A[i] for i=1:rows[1]]...)
o = rows[1]
for j=2:length(rows)
C = hcat([A[i+o] for i=1:rows[j]]...)
o += rows[j]
B = vcat(B, C)
end
return B
end
=#

function Base.vcat(A::MatElem...)
r = nrows(A[1])
c = ncols(A[1])
R = base_ring(A[1])
for i = 2:length(A)
@assert ncols(A[i]) == c
@assert base_ring(A[i]) == R
r += nrows(A[i])
end
X = zero_matrix(R, r, c)
o = 1
for i = 1:length(A)
for j = 1:nrows(A[i])
X[o, :] = A[i][j, :]
o += 1
end
end
return X
end

gens(L::SimpleNumField{T}) where {T} = [gen(L)]

function gen(L::SimpleNumField{T}, i::Int) where {T}
Expand Down
Loading