Skip to content

test(rocsparse): add ELL triangular solve and SELL SpMV unit tests (ROCM-33114) - #13879

Merged
kliegeois merged 12 commits into
ROCm:developfrom
kliegeois:task/rocm-33114-ell-sell-tests
Nov 5, 2026
Merged

kliegeois merged 12 commits into
ROCm:developfrom
kliegeois:task/rocm-33114-ell-sell-tests

Conversation

@kliegeois

@kliegeois kliegeois commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

JIRA ID : ROCM-33114.

Summary

  • Add device unit tests for ELL triangular solve and SELL SpMV. The YAML coverage misses the slice-size boundaries and a partial last slice.
  • ELL solve covers a lower non-unit diagonal, a unit diagonal whose stored diagonal must be ignored, and a zero pivot. The zero-pivot output is preset to a sentinel and its status must match the reported position. Those cases are built only when the library has ELL TRSV.
  • SELL SpMV covers the small-slice kernel (slice 32 and the 128 limit) and the large-slice kernel (slice 129, and slice 300 for two passes per slice), in both directions, each with a partial last slice; m = 600 also leaves a surplus block in the launch grid. It also covers more than 8 entries per row in the small-slice kernel, slice size 1 (the CSR fallback), non-zero beta in both directions, and a transpose case with n > m. The beta = 0 cases start from a non-zero y.
  • y is allocated with a sentinel tail, so a write past row m fails the test. Out-of-bounds reads (for example by padding threads in the transpose kernels) are not detectable by value checks and need a sanitizer.
  • Comparisons are exact: every value is a small integer or dyadic fraction and partial sums stay below 2^24.
  • This does not change sellmv_device.h. It is based on develop after testintel: fix severe issue in sellmvt_device (created by kliegeoi (@kliegeois)) #13339, which guards the padding-thread read of x in the small-slice transpose kernel.

Test plan

  • Built rocsparse-unit-test-device for gfx1201; EllSv.*:SellMv.* and the full binary pass.
  • Removing row < m from the non-transpose large-slice guard makes SellMv.none_slice_size_boundaries fail.
  • CI

These paths are only covered by YAML, which misses the slice-size boundaries and the partial last slice.
@kliegeois
kliegeois requested a review from a team as a code owner October 16, 2026 22:24
@therock-pr-bot

therock-pr-bot Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ⚠️ Warning Error: Source/code files changed without an accompanying unit test.
Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*).
Current: code file(s) changed: projects/rocsparse/clients/unittests/unit_test_internal_level2_ell_sell.cpp; no test file found
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@codecov

codecov Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #13879      +/-   ##
===========================================
+ Coverage    76.89%   77.02%   +0.14%     
===========================================
  Files         2886     2883       -3     
  Lines       481037   479235    -1802     
  Branches     72377    72023     -354     
===========================================
- Hits        369847   369118     -729     
+ Misses       86832    85868     -964     
+ Partials     24358    24249     -109     
Flag Coverage Δ *Carryforward flag
TensileLite-CPP 48.93% <ø> (ø) Carriedforward from b14d24b
TensileLite-Unit 83.68% <ø> (-0.03%) ⬇️ Carriedforward from b14d24b
hipBLAS 90.62% <ø> (ø) Carriedforward from b14d24b
hipBLASLt 64.43% <ø> (-1.77%) ⬇️ Carriedforward from b14d24b
hipCUB 82.68% <ø> (ø) Carriedforward from b14d24b
hipDNN 86.82% <ø> (-0.03%) ⬇️ Carriedforward from b14d24b
hipFFT 42.00% <ø> (+3.98%) ⬆️ Carriedforward from b14d24b
hipRAND 76.12% <ø> (ø) Carriedforward from b14d24b
hipSOLVER 69.67% <ø> (ø) Carriedforward from b14d24b
hipSPARSE 86.99% <ø> (ø) Carriedforward from b14d24b
rocBLAS 68.72% <ø> (-<0.01%) ⬇️ Carriedforward from b14d24b
rocFFT 48.10% <ø> (+2.05%) ⬆️ Carriedforward from b14d24b
rocRAND 57.30% <ø> (-0.01%) ⬇️ Carriedforward from b14d24b
rocSOLVER 76.74% <ø> (ø) Carriedforward from b14d24b
rocSPARSE 83.54% <ø> (+0.03%) ⬆️
rocSPARSE-integration 74.99% <ø> (+0.14%) ⬆️
rocSPARSE-unit 79.43% <ø> (+0.50%) ⬆️
rocThrust 93.15% <ø> (ø) Carriedforward from b14d24b

*This pull request uses carry forward flags. Click here to find out more.
see 92 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

kliegeois and others added 2 commits October 20, 2026 20:49
…3114)

Drop the file-header note and say in the test comment what the
transpose partial-last-slice case checks and what it does not.
Every input is a small integer or dyadic fraction and every partial sum
stays below 2^24, so the old 1e-4 absolute tolerance was already an exact
comparison for most values. Say so and use EXPECT_EQ. Drop the unused
<cmath> include.
…sition (ROCM-33114)

Preset the zero-pivot output to -2 so a get_output that never writes it is
caught, and require rocsparse_status_success exactly when the position is
-1 and rocsparse_status_zero_pivot otherwise, as testing_sptrsv_ell does.
…s (ROCM-33114)

run_ellsv and run_v2_spmv_sell are void helpers, so a failing ASSERT inside
them only returned from the helper and the test went on to check stale
output. Wrap every call in ASSERT_NO_FATAL_FAILURE.
…-33114)

The m = slice = 256 cases ran one full slice with iter == 1, so neither
half of the large-slice row_in_range guard could be false. Replace them
with slice 128 and 129 at m = 600 and slice 300 at m = 700, in both
directions: the last small-slice size, the first large-slice size with
threads outside the slice, and two passes per slice, each with a partial
last slice; m = 600 also leaves a surplus block in the launch grid.

y now carries a sentinel tail past its length, so a non-transpose write
past row m fails the test.

Add a small-slice case per direction with 10 entries per row, so the
strided loop takes a second step and every reduction level combines real
data. build_bidiagonal_sell becomes build_banded_sell with a depth.
…M-33114)

Every transpose case used beta = 0 on a zero y with m == n, so dropping the
scale_array call before the transpose kernels, or giving it m instead of n,
passed. Seed y with 7 in every beta = 0 case, and add a transpose case with
beta = 3 and n > m, where y[m..n-1] only get the beta scaling.
…CM-33114)

ROCSPARSE_WITH_ELL_TRSV was retired with the 5.1 feature flags, so the EllSv tests were compiled out and only the skip stub ran. Drop the guards and the stub.
@kliegeois
kliegeois requested a review from jsandham November 2, 2026 20:29
…M-33114)

run_ellsv created its sptrsv descriptor up front and destroyed it only on
the last line, so any failed ASSERT_* in between returned early and leaked
it. run_v2_spmv_sell and the EllSv test bodies had the same pattern for
their spmat, dnvec and spmv descriptors.

Hold them in the level2 RAII guards from unit_test_device_level2_common.hpp
(sptrsv_descr_guard, spmv_descr_guard, spmat_guard, dnvec_guard), which the
other level-2 unit tests already use.
@kliegeois
kliegeois merged commit 55a2440 into ROCm:develop Nov 5, 2026
132 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants