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

Merge path doesn't zero out NaNs when beta=0 #2166

Open
brian-kelley opened this issue Apr 4, 2024 · 3 comments
Open

Merge path doesn't zero out NaNs when beta=0 #2166

brian-kelley opened this issue Apr 4, 2024 · 3 comments
Assignees

Comments

@brian-kelley
Copy link
Contributor

In spmv (y := alphaAx + betay) if y contains a nan on input and beta is zero, all its entries should be overwritten with zero rather than multipled by zero (preserving nans) before computing alpha*Ax.

The default native algorithm seems to do this, but merge path doesn't.
We should also test this systematically in the spmv unit tests.

@brian-kelley
Copy link
Contributor Author

I think this is actually the same issue as #2010 which was closed with a workaround, but I'll just leave this open instead of reopening that one

@cwpearson cwpearson self-assigned this Apr 22, 2024
@cwpearson
Copy link
Contributor

I think this is the offending line:

KokkosBlas::scal(y, beta, y);

If I'm reading scal correctly, it just just y = beta * y for this case, so if y is NaN it won't work.

The Test_Sparse_spmv_bsr.hpp file generates random vectors for tests, but those vectors do not include NaN, which is why this passes the tests.

@lucbv
Copy link
Contributor

lucbv commented Apr 30, 2024

Then we probably want to have an if statement to check for beta == 0 and call Kokkos::deep_copy(y, 0); in that case

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

Successfully merging a pull request may close this issue.

3 participants