Fix: openMPI shared mem comm type #1176
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maelk3
added
help wanted
fix
priority: high
holke
requested changes
Jul 23, 2024
There was a problem hiding this comment.
Unfortunately the changes in the MPI data type cannot stay like that.
We deliberately kept sc_MPI_INT8_T (see also cburstedde/libsc#166 and cburstedde/libsc#175)
MPI_INT8 is not the same as MPI_BYTE and we should keep it.
Also sc_MPI_LONG_LONG_INT is not sc_MPI_UNSIGNED_LONG_LONG and looses the unsigned information.
These are not in libsc since sc wants to stay backward compatible with old MPI versions.
We do not need this backward compatibility for t8code.
We should discuss a solution.
holke
requested changes
Jul 23, 2024
src/t8.h
Outdated
| @@ -108,7 +108,7 @@ typedef int64_t t8_gloidx_t; | |||
| /** A type for storing SFC indices */ | |||
| typedef uint64_t t8_linearidx_t; | |||
| /** The MPI datatype of t8_linearidx_t */ | |||
| #define T8_MPI_LINEARIDX sc_MPI_UNSIGNED_LONG_LONG | |||
| #define T8_MPI_LINEARIDX sc_MPI_LONG_LONG_INT | |||
There was a problem hiding this comment.
Must remain UNSIGNED_LONG_LONG
| @@ -671,7 +671,7 @@ t8_default_scheme_hex_c::t8_element_MPI_Pack (t8_element_t **const elements, con | |||
| SC_CHECK_MPI (mpiret); | |||
| mpiret = sc_MPI_Pack (&quads[ielem]->z, 1, sc_MPI_INT, send_buffer, buffer_size, position, comm); | |||
| SC_CHECK_MPI (mpiret); | |||
| mpiret = sc_MPI_Pack (&quads[ielem]->level, 1, sc_MPI_INT8_T, send_buffer, buffer_size, position, comm); | |||
| mpiret = sc_MPI_Pack (&quads[ielem]->level, 1, sc_MPI_BYTE, send_buffer, buffer_size, position, comm); | |||
There was a problem hiding this comment.
All these must remain MPI_INT8
|
@holke Thank you for setting up the context that I missed! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe your changes here:
This PR updates the libsc submodule to the lastest development containing the necessary fix in libsc's CMake build system to close #1109. Some of libsc's types have been deprecated. As a result, modifications had to be made on parts that relied on these MPI types. I think someone with better experience with the SFC schemes can weigh in whether I made the correct modifications.
Closes #1109
All these boxes must be checked by the reviewers before merging the pull request:
As a reviewer please read through all the code lines and make sure that the code is fully understood, bug free, well-documented and well-structured.
General
The reviewer executed the new code features at least once and checked the results manually
The code follows the t8code coding guidelines
New source/header files are properly added to the Makefiles
The code is well documented
All function declarations, structs/classes and their members have a proper doxygen documentation
All new algorithms and data structures are sufficiently optimal in terms of memory and runtime (If this should be merged, but there is still potential for optimization, create a new issue)
Tests
Github action
The code compiles without warning in debugging and release mode, with and without MPI (this should be executed automatically in a github action)
All tests pass (in various configurations, this should be executed automatically in a github action)
If the Pull request introduces code that is not covered by the github action (for example coupling with a new library):
Scripts and Wiki
script/find_all_source_files.scpto check the indentation of these files.Licence
doc/(or already has one)