-
Notifications
You must be signed in to change notification settings - Fork 60
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
eachrow
, eachcol
, and reshape
missing
#1376
Comments
On Tue, May 30, 2023 at 06:52:45AM -0700, Markus Kurtz wrote:
I personally prefer a functional programming style. Thus I am missing some functions to deal with matrices. Notably missing are `eachrow`, `eachcol`, and `reshape`. We could copy the first two from Base as
```julia
Base.eachrow(a::MatrixElem) = (view(a, i, :) for i in axes(a, 1))
Base.eachcol(a::MatrixElem) = (view(a, :, i) for i in axes(a, 2))
```
We can't - well with limited efficiency. Before we do this, can we
possibly come together (Wednesday?) to see what that goal here is?
- we cannot support all of julia efficiently - and I don't know if we
should try to support the interface if performance is poor.
As `view` returns 1×n respectively m×1-matrices (instead of `Vector`s like it does for `Matrix`) the resulting `eachrow` and `eachcol` will accordingly also be somewhat inconsistent to their behavior for `Matrix`, but consistent with all other functions dealing with `MatrixElem`.
- see above: some of the problems.
Reshaping with shared data would need support from the appropriate `MatElem` implementations. A flat copying `reshape` instead would be possible using the existing matrix interface. Just create a `similar` matrix with new size and fill in the values from the old matrix.
- see above: some reshape can sometimes be done efficiently, but not
all due to different storage
- copying breaks some of the reshape promise:
- it is slow
- it incurs a big admin overhead (garbage collector)
- it does not change the original elements/ matrices
What is the goal here? what operations do we need/ want?
…
--
Reply to this email directly or view it on GitHub:
#1376
You are receiving this because you are subscribed to this thread.
Message ID: ***@***.***>
|
Sure, I can come along Thursday or Friday. A usage example for lll(a::Vector{ZZMatrix}) = isempty(a) ? a : frommatrix(lll(asmatrix(a)), size(a[1])...)
asmatrix(v::Vector{T}) :: T where T <: MatElem = reduce(vcat, reshape.(v, 1, :))
frommatrix(a::T, r::Int, c::Int) :: Vector{T} where T <: MatElem = [reshape(x, r, c) for x in eachrow(a)] |
On Wed, May 31, 2023 at 06:35:09AM -0700, Markus Kurtz wrote:
Sure, I can come along Thursday or Friday. A usage example for `eachrow` and `reshape` would be the task to LLL-reduce a `Vector` of matrices:
```julia
lll(a::Vector{ZZMatrix}) = isempty(a) ? a : frommatrix(lll(asmatrix(a)), size(a[1])...)
asmatrix(v::Vector{T}) :: T where T <: MatElem = reduce(vcat, reshape.(v, 1, :))
frommatrix(a::T, r::Int, c::Int) :: Vector{T} where T <: MatElem = [reshape(x, r, c) for x in eachrow(a)]
```
elegant: yes, full marks
efficient: only if you're lucky...
reshape could be done efficient iff the matrix if a plain vanilla Nemo
matrix, ie.
- no swapped rows
- no view
in this case we can do, similar to a view, a constant cost reshape -
with shared data (so no need for frommatrix)
However, the reshape to vector will be row-major, not col-major
in this example, with the current technology, asmatrix copies everything
twice
- once in reshape
- once in vcat
frommatrix as well:
- eachrow (might be lucky and use view)
- reshape
the less-elegant version is much faster...
If we want useful, efficient, row-operations, then I don't think eachrow
is (can be) the solution...
Thinking (r)ref:
you want to swap rows (currently for free)
add multiple of one row to another
to do this via rowslices, I'd have to check carefully if the slices
refer to the same matrix in "all" operations. What is the advantage over
swap_rows or add_scaled_row or similar?
… --
Reply to this email directly or view it on GitHub:
#1376 (comment)
You are receiving this because you commented.
Message ID: ***@***.***>
|
On Wed, May 31, 2023 at 06:35:09AM -0700, Markus Kurtz wrote:
Sure, I can come along Thursday or Friday. A usage example for `eachrow` and `reshape` would be the task to LLL-reduce a `Vector` of matrices:
```julia
lll(a::Vector{ZZMatrix}) = isempty(a) ? a : frommatrix(lll(asmatrix(a)), size(a[1])...)
asmatrix(v::Vector{T}) :: T where T <: MatElem = reduce(vcat, reshape.(v, 1, :))
frommatrix(a::T, r::Int, c::Int) :: Vector{T} where T <: MatElem = [reshape(x, r, c) for x in eachrow(a)]
```
for (formerly) fmpz_mat
```
function Base.reshape(x::ZZMatrix, r::Int, c::Int)
@Assert nrows(x) * ncols(x) == r*c
@Assert r == 1
b = ZZMatrix()
b.view_parent = x
ccall((:fmpz_mat_window_init, libflint), Nothing,
(Ref{ZZMatrix}, Ref{ZZMatrix}, Int, Int, Int, Int),
b, x, 0, 0, r, c)
finalizer(_fmpz_mat_window_clear_fn, b)
return b
end
```
julia> m = ZZ[1 2; 3 4]
[1 2]
[3 4]
ulia> r = reshape(m, 1, 4)
[1 2 3 4]
julia> m[1,1] = 17
17
julia> r
[17 2 3 4]
julia> r[1,3] = -9
-9
julia> m
[17 2]
[-9 4]
Note
- this can only work for r==1 (using window_init)
- it is row major
- I think this would work similarly with all Nemo matrices, however AA (Generic)
would be col-major
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I personally prefer a functional programming style. Thus I am missing some functions to deal with matrices. Notably missing are
eachrow
,eachcol
, andreshape
. We could copy the first two from Base asAs
view
returns 1×n respectively m×1-matrices (instead ofVector
s like it does forMatrix
) the resultingeachrow
andeachcol
will accordingly also be somewhat inconsistent to their behavior forMatrix
, but consistent with all other functions dealing withMatrixElem
.Reshaping with shared data would need support from the appropriate
MatElem
implementations. A flat copyingreshape
instead would be possible using the existing matrix interface. Just create asimilar
matrix with new size and fill in the values from the old matrix.The text was updated successfully, but these errors were encountered: