fixed CR comments

aestriplex · aestriplex · commit f3c9109b4912 · 2026-05-31T18:02:29.000+02:00
diff --git a/arm_compute/core/Coordinates.h b/arm_compute/core/Coordinates.h
@@ -33,6 +33,7 @@
 
 #include <algorithm>
 #include <array>
+#include <vector>
 #include <cstddef>
 
 namespace arm_compute
@@ -55,7 +56,7 @@ class Coordinates : public Dimensions<int>
      * @param[in] coords Vector containing the values to initialize the dimensions.
      */
     template <typename T>
-    constexpr Coordinates(std::vector<T> coords) : Dimensions(coords)
+    constexpr Coordinates(const std::vector<T>& coords) : Dimensions(coords)
     {
     }
 
diff --git a/arm_compute/core/Dimensions.h b/arm_compute/core/Dimensions.h
@@ -32,6 +32,7 @@
 
 #include <algorithm>
 #include <array>
+#include <vector>
 #include <functional>
 #include <limits>
 #include <numeric>
diff --git a/arm_compute/core/IReducibleTensor.h b/arm_compute/core/IReducibleTensor.h
@@ -27,6 +27,8 @@
 
 #include "arm_compute/core/SparseTensor.h"
 
+#include <memory>
+
 namespace arm_compute
 {
 /** Forward declaration of COOTensor and CSRTensor class */
diff --git a/arm_compute/core/SparseTensor.h b/arm_compute/core/SparseTensor.h
@@ -29,12 +29,13 @@
 #include "arm_compute/core/Types.h"
 
 #include <functional>
+#include <memory>
 #include <stdexcept>
 
-typedef std::function<bool(const void *)> predicate_t;
-
 namespace arm_compute
 {
+using predicate_t = std::function<bool(const void *)>;
+
 /** Common base class for all sparse tensors */
 class SparseTensor : public ITensor
 {
@@ -63,7 +64,7 @@ class SparseTensor : public ITensor
     /** Returns the ratio of non-zero elements to the total number of elements */
     float density() const;
     /** Returns the dense volume */
-    uint32_t dense_volume(size_t sparse_dim) const;
+    size_t dense_volume(size_t sparse_dim) const;
     /** Returns the number of non zero elements */
     virtual size_t nnz() const = 0;
     /** Converts the sparse tensor to a dense tensor */
diff --git a/arm_compute/core/TensorFormat.h b/arm_compute/core/TensorFormat.h
@@ -21,8 +21,11 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-#ifndef ACL_ARM_COMPUTE_RUNTIME_TENSOR_FORMAT_H
-#define ACL_ARM_COMPUTE_RUNTIME_TENSOR_FORMAT_H
+#ifndef ACL_ARM_COMPUTE_CORE_TENSOR_FORMAT_H
+#define ACL_ARM_COMPUTE_CORE_TENSOR_FORMAT_H
+
+namespace arm_compute
+{
 
 enum class TensorFormat
 {
@@ -31,4 +34,6 @@ enum class TensorFormat
     CSR   = 2,
 };
 
-#endif // ACL_ARM_COMPUTE_RUNTIME_TENSOR_FORMAT_H
+} //namespace arm_compute
+
+#endif // ACL_ARM_COMPUTE_CORE_TENSOR_FORMAT_H
diff --git a/arm_compute/runtime/CSRTensor.h b/arm_compute/runtime/CSRTensor.h
@@ -64,10 +64,13 @@ class CSRTensor final : public SparseTensor, public IMemoryManageable
     /** The size of each index element */
     static constexpr size_t index_size = sizeof(int32_t);
 
-    /** Convert a dense tensor to sparse tensor with specified sparse dimensions using COO format.
+    /** Convert a dense tensor to a CSR sparse tensor.
      *
-     *  @param[in] tensor
-     *  @param[in] sparse_dim  It should belong to [1, tensor->info->num_dimensions()]
+     *  @note Only 2D tensors with NCHW layout are supported. @p sparse_dim is
+     *        ignored; CSR always uses sparse_dim = 2.
+     *
+     *  @param[in] tensor     Source dense tensor. Must be 2D and NCHW.
+     *  @param[in] sparse_dim Unused. Reserved for future use; must be 2.
      */
     CSRTensor(const ITensor *tensor, size_t sparse_dim);
     /** Convert a dense tensor to a *fully* sparse tensor.
diff --git a/src/core/SparseTensor.cpp b/src/core/SparseTensor.cpp
@@ -25,6 +25,8 @@
 
 #include "arm_compute/core/Error.h"
 
+#include <numeric>
+
 namespace arm_compute
 {
 SparseTensor::SparseTensor(size_t dim, size_t sparse_dim) : _total_dim(dim), _sparse_dim(sparse_dim)
@@ -48,7 +50,7 @@ float SparseTensor::sparsity() const
 
 float SparseTensor::density() const
 {
-    return static_cast<float>(nnz()) / static_cast<float>(info()->total_size());
+    return static_cast<float>(nnz()) / static_cast<float>(info()->tensor_shape().total_size());
 }
 
 size_t SparseTensor::dim() const
@@ -61,10 +63,10 @@ bool SparseTensor::is_hybrid() const
     return dense_dim() > 0;
 }
 
-uint32_t SparseTensor::dense_volume(size_t sparse_dim) const
+size_t SparseTensor::dense_volume(size_t sparse_dim) const
 {
     const auto &ts = info()->tensor_shape();
-    return std::accumulate(ts.begin() + sparse_dim, ts.end(), 1, std::multiplies<int>());
+    return std::accumulate(ts.begin() + sparse_dim, ts.end(), size_t{1}, std::multiplies<size_t>());
 }
 
 bool SparseTensor::has_non_zero_elements(uint8_t *arr, size_t len, size_t element_size, predicate_t is_non_zero) const
diff --git a/src/runtime/COOTensor.cpp b/src/runtime/COOTensor.cpp
@@ -52,7 +52,7 @@ COOTensor::COOTensor(const ITensor *tensor, size_t sparse_dim) : SparseTensor(te
                                  "argument must be in [1,%zu] range. %zu is given",
                                  dim(), sparse_dim);
 
-    const uint8_t     *data = tensor->buffer();
+    const uint8_t     *data = tensor->buffer() + info->offset_first_element_in_bytes();
     const size_t dense_dims = dense_dim();
     const auto   is_nonzero = make_is_nonzero_predicate(info->data_type());
 
@@ -75,18 +75,18 @@ COOTensor::COOTensor(const ITensor *tensor, size_t sparse_dim) : SparseTensor(te
     const size_t   slice_size = step * element_size;
 
     size_t value_byte_size = 0;
-    size_t indices_bytes = 0;
     for(size_t i = 0; i < max_iter; i++)
     {
         const size_t offset = i * slice_size;
         if(has_non_zero_elements(const_cast<uint8_t *>(data + offset), slice_size, element_size, is_nonzero))
         {
             value_byte_size += slice_size;
-            indices_bytes += dim() * sizeof(int32_t);
         }
     }
 
-    _allocator.init(coo_tensor_info(info), value_byte_size, indices_bytes);
+    // Indices are stored in _indices (host vector); no index data is written into
+    // the allocator buffer yet, so pass 0 for indices_bytes.
+    _allocator.init(coo_tensor_info(info), value_byte_size, 0);
     _allocator.allocate();
 
     for(size_t i = 0; i < max_iter; i++)
@@ -181,7 +181,7 @@ std::unique_ptr<ITensor> COOTensor::to_dense()
         for(size_t j = 0; j < dense_vol; ++j)
         {
             const void *value_ptr = block_ptr + j * element_size;
-            uint8_t     *base_ptr = tensor->buffer() + final_offset + j * element_size;
+            uint8_t     *base_ptr = tensor->buffer() + first_elem_offset + final_offset + j * element_size;
 
             std::memcpy(base_ptr, value_ptr, element_size);
         }
@@ -211,13 +211,14 @@ const uint8_t *COOTensor::get_value(Coordinates coords) const
     for(size_t i = 0; i < _indices.size(); ++i)
     {
         const Coordinates &c = _indices[i];
-        bool           match = false;
+        bool           match = true;
 
         for(size_t d = 0; d < coords.num_dimensions(); ++d)
         {
-            if(c[d] == coords[d])
+            if(c[d] != coords[d])
             {
-                match = true;
+                match = false;
+                break;
             }
         }
         if(match)
diff --git a/src/runtime/CSRTensor.cpp b/src/runtime/CSRTensor.cpp
@@ -21,7 +21,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-#include "arm_compute/runtime/COOTensor.h"
+#include "arm_compute/runtime/CSRTensor.h"
 
 #include "arm_compute/core/CoreTypes.h"
 #include "arm_compute/core/Error.h"
@@ -75,7 +75,7 @@ CSRTensor::CSRTensor(const ITensor *tensor, size_t sparse_dim) : SparseTensor(te
             if(is_nonzero(data + element_offset))
             {
                 _col_bytes += index_size;
-                value_byte_size += element_size * dense_volume(sparse_dim);
+                value_byte_size += element_size * dense_volume(this->sparse_dim());
             }
         }
     }
diff --git a/src/runtime/SparseTensorAllocator.cpp b/src/runtime/SparseTensorAllocator.cpp
@@ -70,11 +70,11 @@ SparseTensorAllocator &SparseTensorAllocator::operator=(SparseTensorAllocator &&
         _memory   = std::move(o._memory);
         o._memory = Memory();
 
-        _values_bytes = o._values_bytes;
-        _values_bytes = 0;
+        _values_bytes   = o._values_bytes;
+        o._values_bytes = 0;
 
-        _indices_bytes = o._indices_bytes;
-        _indices_bytes = 0;
+        _indices_bytes   = o._indices_bytes;
+        o._indices_bytes = 0;
 
         ITensorAllocator::operator=(std::move(o));
     }
@@ -145,7 +145,7 @@ Status SparseTensorAllocator::import_memory(void *memory)
     ARM_COMPUTE_RETURN_ERROR_ON(_associated_memory_group != nullptr);
     ARM_COMPUTE_RETURN_ERROR_ON(alignment() != 0 && !arm_compute::utility::check_aligned(memory, alignment()));
 
-    _memory.set_owned_region(std::make_unique<MemoryRegion>(memory, info().total_size()));
+    _memory.set_owned_region(std::make_unique<MemoryRegion>(memory, size_bytes()));
     info().set_is_resizable(false);
 
     return Status{};
diff --git a/tests/validation/UNIT/SparseTensor.cpp b/tests/validation/UNIT/SparseTensor.cpp
@@ -77,7 +77,7 @@ bool tensors_are_equal(const test::Accessor &a, const test::Accessor &b)
         const uint8_t *a_value = static_cast<const uint8_t *>(a(id));
         const uint8_t *b_value = static_cast<const uint8_t *>(b(id));
 
-        equal = are_values_equal(a_value, b_value, a.data_type(), element_size);
+        equal &= are_values_equal(a_value, b_value, a.data_type(), element_size);
     });
 
     return equal;
@@ -191,7 +191,7 @@ DATA_TEST_CASE(ConvertCSRTensorToDense, framework::DatasetMode::ALL, combine(
     ARM_COMPUTE_EXPECT(st->is_hybrid() == is_hybrid, framework::LogLevel::ERRORS);
     ARM_COMPUTE_EXPECT(tensors_are_equal(Accessor(t), Accessor(*td)), framework::LogLevel::ERRORS);
     
-    auto st_zero = t_zero.to_coo_sparse(sparse_dim);
+    auto st_zero = t_zero.to_csr_sparse();
     auto td_zero = st_zero->to_dense();
     ARM_COMPUTE_EXPECT(tensors_are_equal(Accessor(t_zero), Accessor(*td_zero)), framework::LogLevel::ERRORS);
 }

Original file line number	Diff line number	Diff line change
`@@ -33,6 +33,7 @@`
`33`	`33`
`34`	`34`	`#include <algorithm>`
`35`	`35`	`#include <array>`
	`36`	`+#include <vector>`
`36`	`37`	`#include <cstddef>`
`37`	`38`
`38`	`39`	`namespace arm_compute`
`@@ -55,7 +56,7 @@ class Coordinates : public Dimensions<int>`
`55`	`56`	`* @param[in] coords Vector containing the values to initialize the dimensions.`
`56`	`57`	`*/`
`57`	`58`	`template <typename T>`
`58`		`- constexpr Coordinates(std::vector<T> coords) : Dimensions(coords)`
	`59`	`+ constexpr Coordinates(const std::vector<T>& coords) : Dimensions(coords)`
`59`	`60`	`{`
`60`	`61`	`}`
`61`	`62`
Original file line number	Diff line number	Diff line change
`@@ -27,6 +27,8 @@`
`27`	`27`
`28`	`28`	`#include "arm_compute/core/SparseTensor.h"`
`29`	`29`
	`30`	`+#include <memory>`
	`31`	`+`
`30`	`32`	`namespace arm_compute`
`31`	`33`	`{`
`32`	`34`	`/** Forward declaration of COOTensor and CSRTensor class */`
Original file line number	Diff line number	Diff line change
`@@ -25,6 +25,8 @@`
`25`	`25`
`26`	`26`	`#include "arm_compute/core/Error.h"`
`27`	`27`
	`28`	`+#include <numeric>`
	`29`	`+`
`28`	`30`	`namespace arm_compute`
`29`	`31`	`{`
`30`	`32`	`SparseTensor::SparseTensor(size_t dim, size_t sparse_dim) : _total_dim(dim), _sparse_dim(sparse_dim)`
`@@ -48,7 +50,7 @@ float SparseTensor::sparsity() const`
`48`	`50`
`49`	`51`	`float SparseTensor::density() const`
`50`	`52`	`{`
`51`		`- return static_cast<float>(nnz()) / static_cast<float>(info()->total_size());`
	`53`	`+ return static_cast<float>(nnz()) / static_cast<float>(info()->tensor_shape().total_size());`
`52`	`54`	`}`
`53`	`55`
`54`	`56`	`size_t SparseTensor::dim() const`
`@@ -61,10 +63,10 @@ bool SparseTensor::is_hybrid() const`
`61`	`63`	`return dense_dim() > 0;`
`62`	`64`	`}`
`63`	`65`
`64`		`-uint32_t SparseTensor::dense_volume(size_t sparse_dim) const`
	`66`	`+size_t SparseTensor::dense_volume(size_t sparse_dim) const`
`65`	`67`	`{`
`66`	`68`	`const auto &ts = info()->tensor_shape();`
`67`		`- return std::accumulate(ts.begin() + sparse_dim, ts.end(), 1, std::multiplies<int>());`
	`69`	`+ return std::accumulate(ts.begin() + sparse_dim, ts.end(), size_t{1}, std::multiplies<size_t>());`
`68`	`70`	`}`
`69`	`71`
`70`	`72`	`bool SparseTensor::has_non_zero_elements(uint8_t *arr, size_t len, size_t element_size, predicate_t is_non_zero) const`
Original file line number	Diff line number	Diff line change
`@@ -52,7 +52,7 @@ COOTensor::COOTensor(const ITensor *tensor, size_t sparse_dim) : SparseTensor(te`
`52`	`52`	`"argument must be in [1,%zu] range. %zu is given",`
`53`	`53`	`dim(), sparse_dim);`
`54`	`54`
`55`		`- const uint8_t *data = tensor->buffer();`
	`55`	`+ const uint8_t *data = tensor->buffer() + info->offset_first_element_in_bytes();`
`56`	`56`	`const size_t dense_dims = dense_dim();`
`57`	`57`	`const auto is_nonzero = make_is_nonzero_predicate(info->data_type());`
`58`	`58`
`@@ -75,18 +75,18 @@ COOTensor::COOTensor(const ITensor *tensor, size_t sparse_dim) : SparseTensor(te`
`75`	`75`	`const size_t slice_size = step * element_size;`
`76`	`76`
`77`	`77`	`size_t value_byte_size = 0;`
`78`		`- size_t indices_bytes = 0;`
`79`	`78`	`for(size_t i = 0; i < max_iter; i++)`
`80`	`79`	`{`
`81`	`80`	`const size_t offset = i * slice_size;`
`82`	`81`	`if(has_non_zero_elements(const_cast<uint8_t *>(data + offset), slice_size, element_size, is_nonzero))`
`83`	`82`	`{`
`84`	`83`	`value_byte_size += slice_size;`
`85`		`- indices_bytes += dim() * sizeof(int32_t);`
`86`	`84`	`}`
`87`	`85`	`}`
`88`	`86`
`89`		`- _allocator.init(coo_tensor_info(info), value_byte_size, indices_bytes);`
	`87`	`+ // Indices are stored in _indices (host vector); no index data is written into`
	`88`	`+ // the allocator buffer yet, so pass 0 for indices_bytes.`
	`89`	`+ _allocator.init(coo_tensor_info(info), value_byte_size, 0);`
`90`	`90`	`_allocator.allocate();`
`91`	`91`
`92`	`92`	`for(size_t i = 0; i < max_iter; i++)`
`@@ -181,7 +181,7 @@ std::unique_ptr<ITensor> COOTensor::to_dense()`
`181`	`181`	`for(size_t j = 0; j < dense_vol; ++j)`
`182`	`182`	`{`
`183`	`183`	`const void value_ptr = block_ptr + j element_size;`
`184`		`- uint8_t base_ptr = tensor->buffer() + final_offset + j element_size;`
	`184`	`+ uint8_t base_ptr = tensor->buffer() + first_elem_offset + final_offset + j element_size;`
`185`	`185`
`186`	`186`	`std::memcpy(base_ptr, value_ptr, element_size);`
`187`	`187`	`}`
`@@ -211,13 +211,14 @@ const uint8_t *COOTensor::get_value(Coordinates coords) const`
`211`	`211`	`for(size_t i = 0; i < _indices.size(); ++i)`
`212`	`212`	`{`
`213`	`213`	`const Coordinates &c = _indices[i];`
`214`		`- bool match = false;`
	`214`	`+ bool match = true;`
`215`	`215`
`216`	`216`	`for(size_t d = 0; d < coords.num_dimensions(); ++d)`
`217`	`217`	`{`
`218`		`- if(c[d] == coords[d])`
	`218`	`+ if(c[d] != coords[d])`
`219`	`219`	`{`
`220`		`- match = true;`
	`220`	`+ match = false;`
	`221`	`+ break;`
`221`	`222`	`}`
`222`	`223`	`}`
`223`	`224`	`if(match)`
Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@`
`21`	`21`	`* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE`
`22`	`22`	`* SOFTWARE.`
`23`	`23`	`*/`
`24`		`-#include "arm_compute/runtime/COOTensor.h"`
	`24`	`+#include "arm_compute/runtime/CSRTensor.h"`
`25`	`25`
`26`	`26`	`#include "arm_compute/core/CoreTypes.h"`
`27`	`27`	`#include "arm_compute/core/Error.h"`
`@@ -75,7 +75,7 @@ CSRTensor::CSRTensor(const ITensor *tensor, size_t sparse_dim) : SparseTensor(te`
`75`	`75`	`if(is_nonzero(data + element_offset))`
`76`	`76`	`{`
`77`	`77`	`_col_bytes += index_size;`
`78`		`- value_byte_size += element_size * dense_volume(sparse_dim);`
	`78`	`+ value_byte_size += element_size * dense_volume(this->sparse_dim());`
`79`	`79`	`}`
`80`	`80`	`}`
`81`	`81`	`}`