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

Sparse Matrix Tests Refactoring #173

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

cameronrutherford
Copy link
Collaborator

In this PR:

  • Deleted any commented out/unused methods that I noticed in the sparse matrix tests. Includes tests moved to sym sparse matrix tests
  • Moved some methods to implementation-specific tests where appropriate
  • Refactored sparse test matrix test driver
  • Cleaned up exception handling in implementation-specific tests (including in vectorTests that were missed in Dense Matrix Unit Test Refactoring #170)

Once #103 is resolved, this implementation-specific section in testMatrixSparse.cpp should probably be removed, along with a RAJA specific test implemented. A new issue can be created to track this if need be, or implementing this test can be part of closing out #103:

//
// Perform hipoMatrixSparseTriplet specific tests
//
//auto * test_triplet = dynamic_cast<tests::MatrixTestsSparseTriplet *>(&test);
if (dynamic_cast<tests::MatrixTestsSparseTriplet *>(&test))
{
  fail += test.copyRowsBlockFrom(*mxn_sparse, *m2xn_sparse,0, 1, M_global-1, mxn_sparse->numberOfNonzeros()-entries_per_row);
}

I will note that this dynamic cast in the template function will always throw a warning: warning: dynamic_cast of ‘hiop::tests::MatrixTestsRajaSparseTriplet test’ to ‘class hiop::tests::MatrixTestsSparseTriplet*’ can never succeed. It seems like an appropriate solution for the moment.

Copy link
Contributor

@ashermancinelli ashermancinelli left a comment

Choose a reason for hiding this comment

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

Looks pretty good!

tests/LinAlg/matrixTestsSparseTriplet.cpp Show resolved Hide resolved
tests/LinAlg/matrixTestsSparseTriplet.cpp Outdated Show resolved Hide resolved
@nychiang
Copy link
Collaborator

@cameronrutherford Thanks! This looks much better and clearer.

@cnpetra
Copy link
Collaborator

cnpetra commented Jan 30, 2021

@pelesh : is this PR still under development or it only waits for you review?

@pelesh
Copy link
Collaborator

pelesh commented Jan 30, 2021

@pelesh : is this PR still under development or it only waits for you review?

Not quite ready to be merged yet.

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 this pull request may close these issues.

5 participants