kokkos / kokkos/stdBLAS

P1673: `symmetric_matrix_vector_product`: bug in implementation

Open
#147 1 comment 1 reaction 1 assignee View on GitHub

@fnrizzi is already working on this.

Since Feb 14, 2022.

bug
Dominant language
C++
Stars
167
Forks
31
PR merge metrics
No merged PRs in 30d

Description

In here the lines starting at 442 :

  if constexpr (std::is_same_v<Triangle, lower_triangle_t>) {
    for (size_type j = 0; j < A.extent(1); ++j) {
      for (size_type i = j; i < A.extent(0); ++i) {
        const auto A_ij = A(i,j);
        y(i) += A_ij * x(j);
        y(j) += A_ij * x(i);
      }
    }
  }
  else {
    for (size_type j = 0; j < A.extent(1); ++j) {
      for (size_type i = 0; i <= j; ++i) {
        const auto A_ij = A(i,j);
        y(i) += A_ij * x(j);
        y(j) += A_ij * x(i);
      }
    }
  }

should be:

  if constexpr (std::is_same_v<Triangle, lower_triangle_t>) {
    for (size_type j = 0; j < A.extent(1); ++j) {
      for (size_type i = j; i < A.extent(0); ++i) {
        const auto A_ij = A(i,j);
        y(i) += A_ij * x(j);
        if(i != j){
          y(j) += A_ij * x(i);
        }
      }
    }
  }
  else {
    for (size_type j = 0; j < A.extent(1); ++j) {
      for (size_type i = 0; i <= j; ++i) {
        const auto A_ij = A(i,j);
        y(i) += A_ij * x(j);
        if(i != j){
          y(j) += A_ij * x(i);
        }
      }
    }
  }

Otherwise we count twice for that element.
Note the spec says: For i in the domain of y, the mathematical expression for the algorithm is y[i] = the sum of A[i,j] * x[j] for all j in the triangle of A specified by t, plus the sum of A[j,i] * x[j] for all j not equal to i such that j,i is in the domain of A but not in the triangle of A specified by t.

Most likely also the updating overload has same issue, have not checked that yet.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.