Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

P1673: symmetric_matrix_vector_product: bug in implementation #147

Open
fnrizzi opened this issue Feb 14, 2022 · 1 comment
Open

P1673: symmetric_matrix_vector_product: bug in implementation #147

fnrizzi opened this issue Feb 14, 2022 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@fnrizzi
Copy link
Contributor

fnrizzi commented Feb 14, 2022

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.

@fnrizzi fnrizzi added the bug Something isn't working label Feb 14, 2022
@fnrizzi fnrizzi self-assigned this Feb 14, 2022
@fnrizzi fnrizzi changed the title symmetric_matrix_vector_product: bug in implementation P1673: symmetric_matrix_vector_product: bug in implementation Mar 14, 2022
@iburyl
Copy link
Contributor

iburyl commented Aug 26, 2024

@mhoemmen That one should be fixed by my PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants