Add opnorm implementation #375
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #375 +/- ##
==========================================
- Coverage 95.00% 93.19% -1.81%
==========================================
Files 17 20 +3
Lines 1100 1190 +90
==========================================
+ Hits 1045 1109 +64
- Misses 55 81 +26 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tmigot
left a comment
tmigot
left a comment
There was a problem hiding this comment.
Thanks @farhadrclass that will be a great add. I made a first batch of comments
There was a problem hiding this comment.
Pull request overview
This pull request introduces a new opnorm function to efficiently compute the operator 2-norm (largest singular value) for matrices and linear operators. The implementation uses a dispatching strategy that selects LAPACK for small dense matrices, ARPACK for larger matrices with Float32/Float64/ComplexF32/ComplexF64 element types, and TSVD for other element types like BigFloat.
Changes:
- Adds
opnormfunction with type-based dispatch for efficient norm computation - Adds comprehensive tests for Float64 and BigFloat matrix types
- Adds new dependencies: Arpack, GenericLinearAlgebra, and TSVD
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 18 comments.
| File | Description |
|---|---|
| src/utilities.jl | Implements opnorm function with dispatch to opnorm_eig for square matrices and opnorm_svd for rectangular matrices, with fallback to TSVD |
| test/test_opnorm.jl | Adds tests for square and rectangular matrices with Float64 and BigFloat types |
| test/runtests.jl | Integrates new test_opnorm.jl into the test suite |
| Project.toml | Adds Arpack, GenericLinearAlgebra, and TSVD dependencies with version constraints |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated 15 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@farhadrclass All tests fail and the branch must be rebased. We also need to hear from @MohamedLaghdafHABIBOULLAH and @MaxenceGollier. |
Introduces a new opnorm function that efficiently computes the operator 2-norm for matrices and linear operators, dispatching to LAPACK, ARPACK, or TSVD as appropriate. Adds comprehensive tests for opnorm covering both Float64 and BigFloat types, and integrates the new test file into the test suite. Update Project.toml
Added Arpack to Project.toml dependencies and compat section. Updated utilities.jl to import Arpack, preparing for usage of its functionality.
Replaces the opnorm function with estimate_opnorm throughout the codebase for clarity. Updates all relevant documentation, exports, and test files to use the new function name, and renames the test file accordingly. Update utilities.jl Update test_estimate_opnorm.jl
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Dominique <dominique.orban@gmail.com>
farhadrclass
force-pushed
the
opNorm_branch
branch
from
05ba478 to
8b0da8c
Compare
Added Arpack, GenericLinearAlgebra, and TSVD as dependencies in Project.toml with corresponding compat entries. Updated test_estimate_opnorm.jl to use consistent spacing and formatting in test assertions.
|
@dpo, Thanks for pointing it out, |
farhadrclass
dismissed
tmigot’s stale review
I can not find what @tmigot said back in Aug 2025, since we have changed many of the code, but the Github doesn't let me to request a review till I apply the changes (which I can not find). So I am dissmiss it now so I can request a new review from Tangi
tmigot
left a comment
tmigot
left a comment
There was a problem hiding this comment.
Thanks @farhadrclass for the PR, it seems that a couple of choices are performance dependent. Could you add a script to the benchmark folder then?
Refactored _estimate_opnorm to add a specialized dispatch for Hermitian and Symmetric matrices with Float/Complex types, improving clarity and correctness. Simplified comments and retry logic in opnorm_eig and opnorm_svd. Expanded and restructured tests to cover type stability, dispatch paths, and edge cases for various matrix types and element types.
|
@tmigot thanks for review, I have added the changes, |
3 participants
Introduces a new opnorm function that efficiently computes the operator 2-norm for matrices and linear operators, dispatching to LAPACK, ARPACK, or TSVD as appropriate. Adds comprehensive tests for opnorm covering both Float64 and BigFloat types, and integrates the new test file into the test suite.
Update Project.toml