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

Zero diagonals in diagonally dominant matrices generated #678

Closed
seheracer opened this issue Apr 7, 2020 · 4 comments
Closed

Zero diagonals in diagonally dominant matrices generated #678

seheracer opened this issue Apr 7, 2020 · 4 comments

Comments

@seheracer
Copy link
Contributor

seheracer commented Apr 7, 2020

Link:
Lines 179-183, kk_generate_diagonally_dominant_sparse_matrix function,
https://github.com/kokkos/kokkos-kernels/blob/1be37e8aee0ddfee5ad5076f3a049c4c473f5665/src/common/KokkosKernels_IOUtils.hpp

Related Code:

  for(int row=0;row<nrows;row++)
  {
    int varianz = (1.0*rand()/RAND_MAX-0.5)*row_size_variance;
    rowPtr[row+1] = rowPtr[row] + elements_per_row+varianz;
  }

Problem:
According to the last line of the above-given code, kk_generate_diagonally_dominant_sparse_matrix may create matrices with some diagonal entries being zero, becauseelements_per_row+varianz may be equal to 0, so, some rows may end up with all zero entries.

Is this behavior acceptable?

Edit:
Also, the loop in

for(SizeType k=rowPtr[row] ;k<rowPtr[row+1] - 1;k++)
does not check if the newly-computed column index already exists in the current row. We want to fix that, too, right?

@ndellingwood
Copy link
Contributor

Is this behavior acceptable?

@seheracer For non-strictly diagonally dominant matrices a row of 0's would allow for a 0 at the diagonal, I don't see any comments in the files you reference to determine if this was intentionally allowed to occur (I doubt it, I'm guessing there would be added options to intentionally generate such a matrix since this would test corner cases in algorithms).

@ndellingwood
Copy link
Contributor

Based on meeting discussion sounds like the path forward is to modify the code to ensure diagonal entries are present, rather than adding structural zeros as place holders for rows-of-zeros.

@brian-kelley
Copy link
Contributor

@ndellingwood I agree with this, GS tests use this and expect the diagonal values to be nonzero.

@seheracer
Copy link
Contributor Author

Sounds good. I will push the PR today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants