fix (sparse): ensure proper assembly when filling in symmetric matrices with blocks

[jalvesz]

This PR fixes the add_block type-bound-procedure for assembling sparse matrices in the cases the matrix is symmetric.
A check on the storage attribute combined with the current row and col enables deciding to skip or not the (row,col) pair for assembly.

[Copilot]

Pull request overview

This PR fixes a bug in the sparse matrix assembly logic for symmetric matrices. When adding blocks of values to symmetric sparse matrices (stored in lower or upper triangular form), the code now properly skips entries that fall outside the stored triangle, preventing incorrect assembly.

Changes:

• Added a skip function to determine whether a (row, col) pair should be skipped based on matrix symmetry storage
• Updated add_block methods for COO, CSR, CSC, ELL, and SELLC sparse matrix types to use the skip logic
• Added comprehensive test coverage for the symmetric matrix block addition functionality

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/sparse/stdlib_sparse_kinds.fypp	Implements the skip function and updates all sparse matrix type add_block methods to check symmetry storage before assembly
test/linalg/test_linalg_sparse.fypp	Adds new test test_add_block_symmetric_skip to verify correct behavior when assembling symmetric matrices with blocks

---

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.55%. Comparing base (5a4b3bf) to head (c20e9d0).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1111      +/-   ##
==========================================
+ Coverage   68.52%   68.55%   +0.03%     
==========================================
  Files         396      396              
  Lines       12746    12746              
  Branches     1376     1376              
==========================================
+ Hits         8734     8738       +4     
+ Misses       4012     4008       -4     

☔ 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.

[jvdp1]

Overall LGTM. I have a minor suggestion

[perazz]

Thank you for this PR. I also second @jvdp1's observation that there should be one routine per kind, rather than one scope per kind. LGTM otherwise.

[perazz]

Please use the stdlib_kinds, only: lk logical kind

[jalvesz]

I use the smallest logical kind on purpose for efficiency.

[jvdp1]

Could it be defined in stdlib_kinds, mainy as lk_int8? Or could c_bool be used (note that I don't like this idea)?

[jalvesz]

Options to define a lk_int8 parameter:

integer, parameter :: lk_int8 = 1 
integer, parameter :: lk_int8 = int8
integer, parameter :: lk_int8 = c_bool 
integer, parameter :: lk_int8 = selected_logical_kind(1) !> only available from the F2023 standard (too new)

The second one could be a good option.

[perazz]

These tiny edits are meant for consistency with the rest of the library. Performance-wise, I believe in most cases (-O2 at least) the private function will be inlined. Regarding the kind, although they may work, both 1 and int8 refer to integers. On the other hand, c_bool is already part of the stdlib_kinds

[jalvesz]

It is not only a style edit, it does have consequences, even if they might not seem very important. logicals in Fortran can have different sizes, just like integers, which mean that one does ask the compiler to use different register sizes depending on the requested size. Since the default Fortran size for logical is 32bits (8bytes) to correspond to the same size of the default integer, one does end up either implying usage of unnecessary extra memory or a couple of additional operations w.r.t the resulting ASM code.

I played a bit with this on CE https://godbolt.org/z/7aK8KvWrn, I don't want to overblow this thread so I won't put all the details but if one goes and play within, changing the compiler, it is possible to see ever so slightly differences between the ASM code for these two versions (I tried with gfotran, ifort and ifx with -O3)

logical(int8) elemental function skip_1(sym,row,col)
    integer(ilp), intent(in) :: sym, row, col
    skip_1 = (sym == sparse_lower .and. row < col) .or. (sym == sparse_upper .and. row > col)
end function

logical(int32) elemental function skip_2(sym,row,col)
    integer(ilp), intent(in) :: sym, row, col
    skip_2 = (sym == sparse_lower .and. row < col) .or. (sym == sparse_upper .and. row > col)
end function

Now, this can be anecdotal, even transparent, depending on the use case. But just as when defining integers one voluntarily sets a storage size and the compiler "respects" one whishes, it does happens with Fortran logicals.

Maybe what we are missing is to add in the kinds module, not only one lk parameter. But maybe a couple of options. I agree that int# is for integers (even if in practical terms the values are the same). So maybe lk8, lk16, lk32, lk64, and a lk with the default pointing to lk32.