From b6518c90d986b7d65f37aed368fcaeac18adad79 Mon Sep 17 00:00:00 2001
From: "romain.biessy" <romain.biessy@codeplay.com>
Date: Wed, 27 Nov 2024 16:51:49 +0000
Subject: [PATCH] Throw exceptions on unsorted CSR format + tests this

---
 .../sparse_blas_spmv_usm.cpp                   |  5 +++++
 .../backends/rocsparse/rocsparse_handles.hpp   |  6 ++++++
 .../sparse_blas/include/test_common.hpp        | 18 ++++--------------
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/examples/sparse_blas/run_time_dispatching/sparse_blas_spmv_usm.cpp b/examples/sparse_blas/run_time_dispatching/sparse_blas_spmv_usm.cpp
index 69be82745..3c274c2bd 100644
--- a/examples/sparse_blas/run_time_dispatching/sparse_blas_spmv_usm.cpp
+++ b/examples/sparse_blas/run_time_dispatching/sparse_blas_spmv_usm.cpp
@@ -146,6 +146,11 @@ int run_sparse_matrix_vector_multiply_example(const sycl::device& dev) {
     oneapi::mkl::sparse::init_csr_matrix(main_queue, &A_handle, nrows, nrows, nnz,
                                          oneapi::mkl::index_base::zero, ia, ja, a);
 
+    // rocSPARSE backend requires that the property sorted is set when using matrices in CSR format.
+    // Setting this property is also the best practice to get best performance.
+    oneapi::mkl::sparse::set_matrix_property(main_queue, A_handle,
+                                             oneapi::mkl::sparse::matrix_property::sorted);
+
     // Create and initialize dense vector handles
     oneapi::mkl::sparse::dense_vector_handle_t x_handle = nullptr;
     oneapi::mkl::sparse::dense_vector_handle_t y_handle = nullptr;
diff --git a/src/sparse_blas/backends/rocsparse/rocsparse_handles.hpp b/src/sparse_blas/backends/rocsparse/rocsparse_handles.hpp
index feccec96f..3ac985dca 100644
--- a/src/sparse_blas/backends/rocsparse/rocsparse_handles.hpp
+++ b/src/sparse_blas/backends/rocsparse/rocsparse_handles.hpp
@@ -93,6 +93,12 @@ struct matrix_handle : public detail::generic_sparse_handle<rocsparse_spmat_desc
                 "sparse_blas", function_name,
                 "The backend does not support unsorted COO format. Use `set_matrix_property` to set the property `matrix_property::sorted`");
         }
+        if (this->format == detail::sparse_format::CSR &&
+            !this->has_matrix_property(matrix_property::sorted)) {
+            throw mkl::unimplemented(
+                "sparse_blas", function_name,
+                "The backend does not support unsorted CSR format. Use `set_matrix_property` to set the property `matrix_property::sorted`");
+        }
     }
 
     void mark_used() {
diff --git a/tests/unit_tests/sparse_blas/include/test_common.hpp b/tests/unit_tests/sparse_blas/include/test_common.hpp
index 3de3b0ffc..4bed1d666 100644
--- a/tests/unit_tests/sparse_blas/include/test_common.hpp
+++ b/tests/unit_tests/sparse_blas/include/test_common.hpp
@@ -76,20 +76,6 @@ inline std::set<oneapi::mkl::sparse::matrix_property> get_default_matrix_propert
 inline std::vector<std::set<oneapi::mkl::sparse::matrix_property>>
 get_all_matrix_properties_combinations(sycl::queue queue, sparse_matrix_format_t format) {
     auto vendor_id = oneapi::mkl::get_device_id(queue);
-    if (vendor_id == oneapi::mkl::device::nvidiagpu && format == sparse_matrix_format_t::COO) {
-        // Ensure all the sets have the sorted or sorted_by_rows properties
-        return { { oneapi::mkl::sparse::matrix_property::sorted },
-                 { oneapi::mkl::sparse::matrix_property::sorted_by_rows,
-                   oneapi::mkl::sparse::matrix_property::symmetric },
-                 { oneapi::mkl::sparse::matrix_property::sorted,
-                   oneapi::mkl::sparse::matrix_property::symmetric } };
-    }
-    if (vendor_id == oneapi::mkl::device::amdgpu &&
-        (format == sparse_matrix_format_t::COO || format == sparse_matrix_format_t::CSR)) {
-        return { { oneapi::mkl::sparse::matrix_property::sorted,
-                   oneapi::mkl::sparse::matrix_property::symmetric } };
-    }
-
     std::vector<std::set<oneapi::mkl::sparse::matrix_property>> properties_combinations{
         { oneapi::mkl::sparse::matrix_property::sorted },
         { oneapi::mkl::sparse::matrix_property::symmetric },
@@ -99,6 +85,10 @@ get_all_matrix_properties_combinations(sycl::queue queue, sparse_matrix_format_t
     if (format == sparse_matrix_format_t::COO) {
         properties_combinations.push_back({ oneapi::mkl::sparse::matrix_property::sorted_by_rows });
     }
+    if (vendor_id == oneapi::mkl::device::nvidiagpu || vendor_id == oneapi::mkl::device::amdgpu) {
+        // Test without any properties set since for backends for which this is not the default behavior
+        properties_combinations.push_back({});
+    }
     return properties_combinations;
 }