Skip to content

Conversation

@Mahmood-Sinan
Copy link
Contributor

This PR adds a generalized lagtm routine that supports arbitrary values of alpha and beta for the operation:

y = alpha * A * x + beta * y

where A is a tridiagonal matrix. The existing LAPACK lagtm restricts alpha and beta to the set { -1, 0, 1 }, which limits its applicability in several higher-level routines.
The routine can be called as follows:

glagtm(trans, n, nrhs, alpha, dl, d, du, x, ldx, beta, b, ldb)

which is similar to the LAPACK version:

lagtm( trans, n, nrhs, alpha, dl, d, du, x, ldx, beta,b, ldb )

except that alpha and beta can be arbitrary(including complex values).

This new subroutine is placed inside
src/lapack_extended/stdlib_extended_lapack_base.fypp (interface) and
src/lapack_extended/stdlib_extended_lapack.fypp (implementation).

@codecov
Copy link

codecov bot commented Dec 9, 2025

Codecov Report

❌ Patch coverage is 0% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.60%. Comparing base (1f6e3a0) to head (ffe1a6f).

Files with missing lines Patch % Lines
...ecialmatrices/example_specialmatrices_cdp_spmv.f90 0.00% 17 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1068      +/-   ##
==========================================
- Coverage   68.69%   68.60%   -0.10%     
==========================================
  Files         392      393       +1     
  Lines       12693    12710      +17     
  Branches     1377     1377              
==========================================
  Hits         8720     8720              
- Misses       3973     3990      +17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jalvesz
Copy link
Contributor

jalvesz commented Dec 10, 2025

Hi @Mahmood-Sinan, this looks pretty neat! the only thing I would suggest would be to keep the files and folders naming somewhat coherent with respect to the name lapack_extended.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a generalized glagtm routine that extends LAPACK's lagtm function to support arbitrary scalar values for alpha and beta (not just -1, 0, 1), including complex values. This enables the operation y = alpha * A * x + beta * y for tridiagonal matrices with greater flexibility.

Key Changes:

  • Implemented new glagtm subroutine supporting arbitrary alpha/beta values for real and complex types
  • Updated spmv interface to accept complex alpha/beta parameters (previously restricted to real)
  • Added fallback logic to use standard LAPACK lagtm when alpha/beta are special values (-1, 0, 1) for performance

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/lapack_extended/stdlib_extended_lapack_base.fypp Interface definition for new glagtm subroutine
src/lapack_extended/stdlib_extended_lapack.fypp Implementation of glagtm routine handling N/T/C transpose operations
src/lapack_extended/CMakeLists.txt Build configuration for new lapack_extended module
src/CMakeLists.txt Integration of lapack_extended module into main build
src/stdlib_specialmatrices.fypp Updated interface signatures to accept complex alpha/beta
src/stdlib_specialmatrices_tridiagonal.fypp Logic to dispatch between lagtm and glagtm based on parameter values
test/linalg/test_linalg_specialmatrices.fypp Added tests for random alpha/beta values across N/T/H operations
example/specialmatrices/example_specialmatrices_cdp_spmv.f90 Example demonstrating complex alpha/beta usage
example/specialmatrices/CMakeLists.txt Build configuration for new example

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

jalvesz and others added 2 commits December 10, 2025 18:47
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Mahmood-Sinan
Copy link
Contributor Author

Hi @Mahmood-Sinan, this looks pretty neat! the only thing I would suggest would be to keep the files and folders naming somewhat coherent with respect to the name lapack_extended.

Sure, I will do that in the next commit.

@Mahmood-Sinan
Copy link
Contributor Author

I also noticed that the documentation in
doc/specs/stdlib_specialmatrices.md contains the following warning:

@warning
Due to limitations of the underlying `lapack` driver, currently `alpha` and `beta` can only take one of the values [-1, 0, 1] for `tridiagonal` and `symtridiagonal` matrices. See `lagtm` for more details.
@endwarning

Since the specialmatrices routines now call glagtm when alpha or beta take arbitrary values, this warning is no longer entirely accurate. Should I update this as well?

@Mahmood-Sinan
Copy link
Contributor Author

I’ve applied the changes. Please let me know if anything else is needed.

Copy link
Contributor

@jalvesz jalvesz left a comment

Choose a reason for hiding this comment

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

LGTM @Mahmood-Sinan, thanks for this new contribution!

@jalvesz
Copy link
Contributor

jalvesz commented Dec 17, 2025

I don't really understand why the codecov jobs are failing. It would be good to understand this, maybe @loiseaujc could have an insight here?

@jalvesz jalvesz requested a review from jvdp1 December 17, 2025 05:39
Copy link
Member

@jvdp1 jvdp1 left a comment

Choose a reason for hiding this comment

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

Overall LGTM. I have only a minor question

@Mahmood-Sinan
Copy link
Contributor Author

@jalvesz Thanks for the review.

@jvdp1
Copy link
Member

jvdp1 commented Dec 17, 2025

@loiseaujc @jalvesz @Mahmood-Sinan it seems that the covecod failed because the Fortran example is not covered (report here). how should we deal with that?

@Mahmood-Sinan
Copy link
Contributor Author

@loiseaujc @jalvesz @Mahmood-Sinan it seems that the covecod failed because the Fortran example is not covered (report here). how should we deal with that?

The example example_specialmatrices_cdp_spmv can be removed if you prefer, because there is already another example example_specialmatrices_dp_spmv for spmv interface. I kept it because example_specialmatrices_dp_spmv calls LAPACK lagtm and example_specialmatrices_cdp_spmv calls lapack_extended glagtm internally.

@jalvesz
Copy link
Contributor

jalvesz commented Dec 18, 2025

I don't think removing the example is the solution. It would seem (and this is just a hunch) that the coverage tool is targeting the examples as sources to be analyzed in terms of coverage. The only sources to be "targeted" should be the sources within src and the test and example (or without the examples) should be the point of comparison for the coverage.

@jvdp1
Copy link
Member

jvdp1 commented Dec 18, 2025

I don't think removing the example is the solution. It would seem (and this is just a hunch) that the coverage tool is targeting the examples as sources to be analyzed in terms of coverage. The only sources to be "targeted" should be the sou

I agree, we should keep it. Is there a way to avoid running the examples with fpm test?

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.

3 participants