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

Heat Transfer Example: MPI Datatype #3593

Merged
merged 1 commit into from
Apr 28, 2023

Conversation

ax3l
Copy link
Contributor

@ax3l ax3l commented Apr 25, 2023

Replace the MPI_REAL8 datatype, which is optional in the MPI standard and not always available, with MPI_DOUBLE. The latter is always available and corresponds to the data sent around.

First seen as an OpenMPI compile error on a user's machine:
openPMD/openPMD-api#1424 (comment)

I found no further examples that use the same MPI type and would need an update.

Replace the `MPI_REAL8` datatype, which is optional in the MPI
standard and not always available, with `MPI_DOUBLE`. The latter
is always available and corresponds to the data sent around.
@eisenhauer
Copy link
Member

@vicentebolea What is your preferred protocol for maintaining 2.9 and master branches currently? I've submitted some things (like the DAOS engine) to master because we clearly don't want them in a release any time soon. Others, like PR #3591 are clear bug fixes related to new things in the release (default to BP5) and probably need to be in the release (and backported to master). Is this PR similar? Or more generally should most everything go to release_29 and be tagged for backporting to master? I.E. every kind of "normal" fix that isn't a feature that we don't want in 2.9 (which I guess is most every PR).

@vicentebolea
Copy link
Collaborator

What is your preferred protocol for maintaining 2.9 and master branches currently?

We should aim to keep the release_XY branches API/ABI compatible with very counted exceptions, this is, only bugfixes should go to the release branch and they should be carefully chosen so that they dont change either the API (ABI). The rationale of being strict with it is that downstream they expect it to be like this. They expect not to recompile when changing the patch version of adios2.

This PR, for instance looks fine, change the function call of mpi procedures no change in our API/ABI.

#3591 is a bigger change a deeper analysis is needed. There is a high chance of failure doing this manually, so I think that in the future we would want to add automated test for checking that API/ABI compatibility maybe something like running/building ADIOS2 tests from the X.Y.0 version using the source code of the new PR (API) or just linking with the new binaries (ABI) of the pull-request.

On the other hand we will have one more release before the end of September.

Vicente

@vicentebolea
Copy link
Collaborator

Also this branch can be rebased to release_29 for easier backporting.

@eisenhauer
Copy link
Member

#3591 is a bigger change a deeper analysis is needed. There is a high chance of failure doing this manually, so I think that in the future we would want to add automated test for checking that API/ABI compatibility maybe something like running/building ADIOS2 tests from the X.Y.0 version using the source code of the new PR (API) or just linking with the new binaries (ABI) of the pull-request.

Thanks Vicente. I'll followup in #3591.

Copy link
Contributor

@anagainaru anagainaru left a comment

Choose a reason for hiding this comment

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

I assume this was not approved because it's not rebased on release_29?

@eisenhauer
Copy link
Member

I assume this was not approved because it's not rebased on release_29?

Yeah, at least on my end. @ax3l, can you rebase and retarget to the release_29 branch?

@vicentebolea
Copy link
Collaborator

Alternatively, we can merge to master this and I can backport it later to release. @ax3l is a good long time contributor but not a core developer, so we can give him a hand on this :). @anagainaru if you approve Ill follow up the backport.

@vicentebolea vicentebolea merged commit 50aa9a3 into ornladios:master Apr 28, 2023
@vicentebolea vicentebolea self-assigned this May 2, 2023
vicentebolea added a commit to vicentebolea/ADIOS2 that referenced this pull request May 3, 2023
Heat Transfer Example: MPI Datatype

(cherry picked from commit 50aa9a3)
vicentebolea added a commit that referenced this pull request May 3, 2023
Backport:  Heat Transfer Example: MPI Datatype #3593
@ax3l ax3l deleted the fix-mpireal8-example branch May 4, 2023 03:56
@ax3l
Copy link
Contributor Author

ax3l commented May 4, 2023

Thanks for the help!

Sorry for being a bit slow, extended proposal season and deadlines kept me busy.

Thanks for the help backporting. Otherwise, if a simple rebase helps, please do not hesitate to rebase my PRs and force push on it :)

I admit my own project's backports are usually cherry-picked, your workflow is more elaborate and I sometimes forget that :)

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.

4 participants