Zoltan2: Restore Zoltan2 test and Kokkos unordered map #4266
Conversation
Using Host Space unordered map for now to avoid prior cuda issues. Switching back to Kokkos anticipating we'll be developing parallel forms for this later. Also restores a new Zoltan2 test to working status. Also cleans up some of the murmur hash code to avoid the global function declarations.
|
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
| @@ -595,8 +538,7 @@ int Zoltan2_Directory<gid_t,lid_t,user_t>::update_local( | |||
| node.owner = owner; | |||
| node.errcheck = owner; | |||
|
|
|||
| if(!node_map.insert({*gid, node}).second) { | |||
| // if(node_map.insert(*gid, node).failed()) { // TODO Switch to Kokkos map | |||
| if(node_map.insert(*gid, node).failed()) { | |||
| // Need more nodes (that's the assumption here if we have failure) | |||
There was a problem hiding this comment.
failed() isn't the only possible non-successful result; see freed_existing in Kokkos_UnorderedMap.hpp.
There was a problem hiding this comment.
I think freed_existing() is only relevant for parallel or threaded cases? I can add this when we develop parallel forms.
| { | ||
| const Zoltan2_Directory_Node<gid_t,lid_t,user_t> & node = | ||
| node_map.at(*gid); | ||
| // node_map.value_at(node_index); // TODO: Switch to Kokkos map | ||
| node_map.value_at(node_index); | ||
| /* matching global ID found! Return gid's information */ | ||
| if(lid) { |
There was a problem hiding this comment.
Not a fan of a pointer being called lid. That confused me until I saw the next line.
There was a problem hiding this comment.
Agreed. Some code is still replicated from zoltan and as I continue to work on it, I need to convert the original c code to be more c++ like. We plan to return to this to add parallel forms. I will improve it.
| @@ -1084,15 +1023,15 @@ int Zoltan2_Directory<gid_t,lid_t,user_t>::remove( | |||
| // eliminating remove_local and just calling here but will keep for now to | |||
| // keep symmetry with other modes. | |||
| if(nrec > 0) { | |||
| // node_map.begin_erase(); // TODO Switch to Kokkos Map | |||
| node_map.begin_erase(); | |||
| for (int i = 0; i < nrec; i++) { | |||
| msg_t *ptr = reinterpret_cast<msg_t*>(&(rbuff[i*remove_msg_size])); | |||
There was a problem hiding this comment.
Does this obey ANSI strict aliasing rules? (Remember the memcpy line of code I showed a little while ago?)
There was a problem hiding this comment.
// no strict-alias warning
{
char char_ptr[10];
int * ptr = reinterpret_cast<int *>(char_ptr);
}
// no strict-alias warning
{
char * char_ptr = new char[10];
int * ptr = reinterpret_cast<int *>(char_ptr);
}
// strict-alias warning
{
char char_ptr[10];
int * ptr;
*ptr = *(reinterpret_cast<int *>(char_ptr));
}
// no strict-alias warning
{
char * char_ptr = new char[10];
int * ptr;
*ptr = *(reinterpret_cast<int *>(char_ptr));
}
|
Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
|
Status Flag 'Pre-Test Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ kddevin ]! |
|
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3_SERIAL
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_9.2
Jenkins Parameters
Using Repos:
Pull Request Author: MicheldeMessieres |
|
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3_SERIAL
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_9.2
Jenkins Parameters
|
|
Status Flag 'Pre-Merge Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ kddevin ]! |
|
Status Flag 'Pull Request AutoTester' - AutoMerge IS ENABLED, but the Label AT: AUTOMERGE is not set. Either set Label AT: AUTOMERGE or manually merge the PR... |
@trilinos/zoltan2
Motivation and Context
This is a follow up PR to clean up a few issues from PR #4252.
It restores a missing zoltan2 test.
It restored the directory to use Kokkos unordered map (now host space) anticipating we'll be implementing parallel forms in the future.
It cleans up some of the directory hash code to avoid a global static method definition.
How Has This Been Tested?
Tested on white cuda.
Tested on mac clang parallel and serial.
Checklist