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

Fix bug with mkl impl of spgemm #1167

Merged
merged 1 commit into from
Nov 2, 2021
Merged

Fix bug with mkl impl of spgemm #1167

merged 1 commit into from
Nov 2, 2021

Conversation

ndellingwood
Copy link
Contributor

No description provided.

@ndellingwood
Copy link
Contributor Author

Matching fix to trilinos/Trilinos#9891.
This is only needed for master branch, was resolved on develop branch by PR #1135 with the clang-format enhancement which was not included in 3.5.0 release

@ndellingwood ndellingwood mentioned this pull request Nov 2, 2021
Merged
e10harvey
e10harvey requested changes Nov 2, 2021
View reviewed changes
Copy link
Contributor

@e10harvey e10harvey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

THanks, @ndellingwood ! Please see questions below.

src/sparse/impl/KokkosSparse_spgemm_mkl_impl.hpp
@@ -425,12 +424,6 @@ void mkl_symbolic(
MKL\n"); return;
}
*/
if (std::is_same<idx, int>::value) {
Copy link
Contributor

@e10harvey e10harvey Nov 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see where this if-block ends; is there a matching } for it? Should the if-block just be converted to a block -- {.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@e10harvey there is not a matching brace to close this if block, that was what was causing the compilation error. There was an apparent copy+paste error in a past update to this file that duplicated the lines of code that I removed in this PR (and so removing the mismatch in open-closed braces)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@e10harvey See lines 433-448 of this file which had mistaken duplication to lines 428-431, removed by this PR

@e10harvey e10harvey self-requested a review November 2, 2021 18:29
@e10harvey
Copy link
Contributor

@ndellingwood, @lucbv: Shall I enable the auto-tester for pull requests targeting master?

@ndellingwood
Copy link
Contributor Author

ndellingwood commented Nov 2, 2021 •
edited
Loading

Alright, looks like the issue was never on the develop branch but had to do with problems with a cherry-pick to the release-candidate. The duplication of lines causing the compilation error occurred with commit 0598690 , here is the diff 0598690

@ndellingwood ndellingwood merged commit 92d1e94 into kokkos:master Nov 2, 2021
@ndellingwood ndellingwood deleted the fix-mklspgemm branch November 2, 2021 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@e10harvey e10harvey e10harvey approved these changes

@brian-kelley brian-kelley Awaiting requested review from brian-kelley

@lucbv lucbv Awaiting requested review from lucbv

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ndellingwood @e10harvey