-
Notifications
You must be signed in to change notification settings - Fork 14
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
Add neighbor list unit test #64
Conversation
dcadc10
to
dd70f51
Compare
@@ -46,4 +50,7 @@ macro(CabanaMD_add_tests) | |||
endforeach() | |||
endmacro() | |||
|
|||
CabanaMD_add_tests(MPI NAMES Integrator) | |||
CabanaMD_add_tests(NAMES Integrator Neighbor) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will you add the MPI
case back here for tests that require multiple ranks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that should only be the Comm class
|
||
//---------------------------------------------------------------------------// | ||
template <class ListType, class PositionSlice> | ||
void checkFullNeighborListPartialRange( const ListType &list, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if we can combine the full and partial test to reduce code here but I think this is OK for now as it is essentially what I implemented in Cabana
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I should do that both here and in Cabana
These tests are very similar to those in Cabana.
This also ensures that we don't test with more than one MPI rank (except for the future
Comm
test) and updates unit_test file license years.One more towards #44