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.

Sign up for GitHub

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

Jump to bottom

CrsMatrix: sumIntoValues and replaceValues incorrectly count the number of valid column indices. #11

Closed
vbrunini opened this issue Mar 10, 2017 · 12 comments
Closed

CrsMatrix: sumIntoValues and replaceValues incorrectly count the number of valid column indices. #11

vbrunini opened this issue Mar 10, 2017 · 12 comments
Assignees
@mhoemmen
Labels
bug fixed-in-develop bug InDevelop

Comments

@vbrunini
Copy link

I believe in both cases the ++numValid; line needs to be within the if(offset != length) branch, right now it is executed unconditionally.
@aprokop
Copy link
Contributor

aprokop commented Mar 10, 2017

Looking at older Tpetra implementation, this is the case. Both it and hint are inside the if (offset != length) branch.

@mhoemmen
Copy link
Contributor

@vbrunini oops! sorry about that :-)

@crtrott
Copy link
Member

crtrott commented Apr 28, 2017

Mark did you fix this?

@crtrott crtrott added the bug label Apr 28, 2017
@mhoemmen
Copy link
Contributor

@crtrott Not yet; too many things on my plate.

@crtrott
Copy link
Member

crtrott commented Sep 12, 2017

Is this actually fixed?

@mhoemmen
Copy link
Contributor

@crtrott No. I can take it if you haven't started yet.

@mhoemmen
Copy link
Contributor

I'm on it.

mhoemmen pushed a commit to mhoemmen/Trilinos that referenced this issue Sep 19, 2017
kokkos-kernels: Fix replaceValues / sumIntoValues bug
5c903c6 
The bug in question is the following:

kokkos/kokkos-kernels#11

I will fix this in Trilinos first, so that Trilinos users can see the
benefit of this fix immediately.  Once Trilinos tests pass, I'll add
the patch to kokkos-kernels.
mhoemmen pushed a commit that referenced this issue Sep 20, 2017
Fix #11 (replaceValues / sumIntoValues bug)
14f071f
@mhoemmen mhoemmen mentioned this issue Sep 20, 2017
Merged
@mhoemmen
Copy link
Contributor

@mndevec I submitted a PR, #88. Please review; thanks! It's ready to merge if you approve.

@mhoemmen
Copy link
Contributor

@mndevec FYI I tested the PR through Trilinos already.

@crtrott
Copy link
Member

crtrott commented Sep 20, 2017

Did you test with CUDA?

crtrott added a commit that referenced this issue Sep 20, 2017
@crtrott
Merge pull request #88 from kokkos/Fix-11
ba2d3dd 
Fix #11 (replaceValues / sumIntoValues bug)
@crtrott
Copy link
Member

crtrott commented Sep 20, 2017

NVM it is so simple I just merge it.

@mhoemmen
Copy link
Contributor

@crtrott wrote:

Did you test with CUDA?

Yes, at least Tpetra. There are some Stokhos tests that were failing before for me; this is not relevant to that. Thanks!

lxmota pushed a commit to trilinos/Trilinos that referenced this issue Sep 21, 2017
@lxmota
kokkos-kernels: Fix replaceValues / sumIntoValues bug
caceb85 
The bug in question is the following:

kokkos/kokkos-kernels#11

I will fix this in Trilinos first, so that Trilinos users can see the
benefit of this fix immediately.  Once Trilinos tests pass, I'll add
the patch to kokkos-kernels.
kyungjoo-kim pushed a commit that referenced this issue Dec 11, 2017
@kyungjoo-kim
Fix #11 (replaceValues / sumIntoValues bug)
65db02e
@crtrott crtrott closed this as completed Dec 15, 2017
@crtrott crtrott mentioned this issue Dec 15, 2017
Merged
9 tasks
@e10harvey e10harvey mentioned this issue May 7, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mhoemmen mhoemmen

Labels
bug fixed-in-develop bug InDevelop
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@mhoemmen @aprokop @vbrunini @crtrott