Skip to content

Add mul! for matrices#330

Merged
amontoison merged 3 commits intoJuliaSmoothOptimizers:mainfrom
gdalle:matmat
May 16, 2024
Merged

Add mul! for matrices#330
amontoison merged 3 commits intoJuliaSmoothOptimizers:mainfrom
gdalle:matmat

Conversation

@gdalle
Copy link
Copy Markdown
Contributor

@gdalle gdalle commented May 16, 2024

Goal: to fix #322 by adding mul!(res::AbstractMatrix, op::LinearOperator, m::AbstractMatrix)

In practice, there are lots of things to overload and I don't know if you want to do all of them. I also don't know how you want to test this new functionality. But anyway, here's a first draft (without tests) to get the discussion started

@amontoison
Copy link
Copy Markdown
Member

I think we should only overload mul!, transpose and adjoint for now. We support the other ones later.

@gdalle
Copy link
Copy Markdown
Contributor Author

gdalle commented May 16, 2024

Okay, how about the 5-argument mul!? From what I can tell, it uses pre-allocated storage that is specific to operator-vector multiplication?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented May 16, 2024

Package name latest stable
CaNNOLeS.jl
DCISolver.jl
FletcherPenaltySolver.jl
JSOSolvers.jl
Krylov.jl
NLPModels.jl
NLPModelsModifiers.jl
PROPACK.jl
Percival.jl
QuadraticModels.jl
SolverTools.jl

@amontoison
Copy link
Copy Markdown
Member

We can just support the 3-args mul! for now with operator-matrix products. It's only what we need for Krylov.jl.

@gdalle gdalle marked this pull request as ready for review May 16, 2024 12:40
@gdalle
Copy link
Copy Markdown
Contributor Author

gdalle commented May 16, 2024

I added the minimal amount of changes and the minimal tests, can you approve the workflows?

@gdalle
Copy link
Copy Markdown
Contributor Author

gdalle commented May 16, 2024

@amontoison at your disposal to add more code, tests or docs depending on what you see fit in the review

@amontoison
Copy link
Copy Markdown
Member

amontoison commented May 16, 2024

Is it working with block_gmres in Krylov.jl?

@gdalle
Copy link
Copy Markdown
Contributor Author

gdalle commented May 16, 2024

Yes!

julia> using Krylov, LinearOperators

julia> A, B = rand(2, 2), rand(2, 2);

julia> block_gmres(A, B)
([0.711998925881128 -0.31790146670357355; 1.0257506949284791 3.3662342660807827], SimpleStats
 niter: 1
 solved: true
 inconsistent: false
 residuals: []
 Aresiduals: []
 κ₂(A): []
 timer: 189.24μs
 status: solution good enough given atol and rtol
)

julia> block_gmres(LinearOperator(A), B)
([0.711998925881128 -0.31790146670357355; 1.0257506949284791 3.3662342660807827], SimpleStats
 niter: 1
 solved: true
 inconsistent: false
 residuals: []
 Aresiduals: []
 κ₂(A): []
 timer: 521.58ms
 status: solution good enough given atol and rtol
)

CompositeLinearOperator(T, nrow, ncol, symm, herm, prod!, tprod!, ctprod!, args5, S = S)
end

# TODO: overload the above for matrices?
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you remove this comment and open the issue for some functionalities that are not supported with matrices?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I will merge the PR after that 🙂

Copy link
Copy Markdown
Contributor Author

@gdalle gdalle May 16, 2024

Choose a reason for hiding this comment

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

Comment removed, issue opened in #331

@amontoison amontoison merged commit 3841b88 into JuliaSmoothOptimizers:main May 16, 2024
@gdalle gdalle deleted the matmat branch May 17, 2024 05:03
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.

Missing mul!(::Matrix, ::LinearOperator, ::Matrix)

2 participants