-
Notifications
You must be signed in to change notification settings - Fork 64
add support for Zoltan with PT-Scotch #478
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
base: develop
Are you sure you want to change the base?
Conversation
As is, this changes the CMake build process to need the ENABLE_PARMETIS flag. However, |
Checking with my double cone case in capstone. |
@bobpaw Is this ready for review? |
@bobpaw Can you merge develop or rebase on develop? I just pushed a change that will fix this CI error: |
- zoltan/CMakeLists.txt: add ENABLE_PARMETIS/ENABLE_PTSCOTCH flags. - change include_dirs/link_libraries to depend on ENABLE_ flags. - cmake/FindZoltan.cmake: remove default path when ZOLTAN_PREFIX is defined. - search for pt-scotch or parmetis depending on what is enabled. - zoltan/apfZoltan.h: add PTSCOTCH ZoltanMethod. - zoltan/apfZoltanCallbacks.cc: make PARMETIS/PTSCOTCH options only work when support is compiled in. - make GRAPH method prefer ParMETIS, then PT-Scotch, then PHG. - guard PARMETIS-specifics. Signed-off-by: Aiden Woodruff <[email protected]>
- cmake/FindZoltan.cmake: fix find_path/find_library when ZOLTAN_PREFIX not specified. since it is always defined, check instead for truthiness. - add FATAL_ERROR if Zoltan is not found. - add debug message with found Zoltan location. - find Zoltan_config.h and search for #define HAVE_PARMETIS and #define HAVE_SCOTCH. use the results to determine which libraries are required. - set the PUMI_HAS_PARMETIS and PUMI_HAS_PTSCOTCH from this file. - zoltan/CMakeLists.txt: remove ENABLE_PARMETIS and ENABLE_PTSCOTCH cache options. - replace ENABLE_PTSCOTCH/ENABLE_PARMETIS with PUMI_HAS_PARMETIS/PUMI_HAS_PTSCOTCH which are detected in FindZoltan.cmake. Signed-off-by: Aiden Woodruff <[email protected]>
7aa4adb
to
ef13770
Compare
@cwsmith I think it should be good but I haven't gotten a chance to test on my large test cases yet. |
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.
Looks good. Thank you. A couple comments are below.
#elif defined(PUMI_HAS_PTSCOTCH) | ||
Zoltan_Set_Param(ztn, "GRAPH_PACKAGE", "Scotch"); | ||
#else | ||
// Zoltan_Set_Param(ztn, "HYPERGRAPH_PACKAGE", "PHG"); //FIXME: not required? |
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.
It looks like we are providing at least some of the hypergraph callbacks
core/zoltan/apfZoltanCallbacks.cc
Lines 363 to 364 in 17ad033
Zoltan_Set_Fn(ztn, ZOLTAN_HG_SIZE_CS_FN_TYPE, (void (*)()) getHgSize, (void*) (zb)); | |
Zoltan_Set_Fn(ztn, ZOLTAN_HG_CS_FN_TYPE, (void (*)()) getHg, (void*) (zb)); |
so I think this should work as a decent fallback.
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 think even if the hypergraph functions are not fully implemented we can use PHG and the hypergraph will be constructed using the graph query operations.
https://sandialabs.github.io/Zoltan/ug_html/ug_graph_vs_hg.html
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.
OK. Thank you.
So, whatever we do for the above 'warnings' (i.e., GRAPH requested but PHG provided) we should do that here in the #else
block.
https://sandialabs.github.io/Zoltan/ug_html/ug_alg_hypergraph.html
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.
A few more items. Sorry for the churn.
@@ -29,9 +31,33 @@ static int setZoltanLbMethod(struct Zoltan_Struct* ztn, ZoltanMesh* zb) | |||
case HYPERGRAPH: | |||
lbMethod = "HYPERGRAPH"; break; | |||
case PARMETIS: //fall into GRAPH settings |
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.
we should remove the 'fall into GRAPH settings' comment
lion_oprint(1, "WARNING: ParMETIS ZoltanMethod requested but ParMETIS" | ||
" was not enabled at build time.\n"); |
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 think we should make this an error (i.e., stop execution) or, indicate to the user that PHG will be used instead (IIUC):
https://sandialabs.github.io/Zoltan/ug_html/ug_alg_graph.html
Also, lion_oprint
will write on all ranks; we should probably only print from rank 0 here.
lion_oprint(1, "WARNING: PT-Scotch ZoltanMethod requested but PT-Scotch" | ||
" was not enabled at build time.\n"); |
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 think we should make this an error (i.e., stop execution) or, indicate to the user that PHG will be used instead (IIUC):
https://sandialabs.github.io/Zoltan/ug_html/ug_alg_graph.html
Also, lion_oprint will write on all ranks; we should probably only print from rank 0 here.
#elif defined(PUMI_HAS_PTSCOTCH) | ||
Zoltan_Set_Param(ztn, "GRAPH_PACKAGE", "Scotch"); | ||
#else | ||
// Zoltan_Set_Param(ztn, "HYPERGRAPH_PACKAGE", "PHG"); //FIXME: not required? |
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.
OK. Thank you.
So, whatever we do for the above 'warnings' (i.e., GRAPH requested but PHG provided) we should do that here in the #else
block.
https://sandialabs.github.io/Zoltan/ug_html/ug_alg_hypergraph.html
/runtests |
Build Log |
@cwsmith is that hosted run with the normal (parmetis) Zoltan? |
Yes. |
Note to self. Example from OpenFOAM: https://bugs.openfoam.org/view.php?id=2032 |
add support for Zoltan with PT-Scotch
This allows for Zoltan which doesn't depend on ParMETIS exclusively. adds CMake support and alter the apfZoltanCallbacks slightly to prefer ParMETIS, then PT-SCOTCH, then PHG (depending on what is available).