Distinguish all char types in ADIOS2 backend #1275
Merged
Conversation
This file contains hidden or 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
franzpoeschel
force-pushed
the
fix-adios2-char-datatype
branch
from
3d953e8 to
ac39140
Compare
franzpoeschel
commented
May 16, 2022
franzpoeschel
force-pushed
the
fix-adios2-char-datatype
branch
from
ac39140 to
705a8ac
Compare
12 tasks
Contributor
Author
|
Note: We should think about introducing a copied Datatype enum in the JSON backend as well. In the JSON backend, the string representations of the datatypes are part of the written dataset on disk, so having that JSON-specific will help not accidentally introducing a breaking change. |
ax3l
reviewed
May 24, 2022
franzpoeschel
force-pushed
the
fix-adios2-char-datatype
branch
2 times, most recently
from
f0e357c to
b3243dd
Compare
Contributor
Author
|
I now removed the ADIOS2Datatype thing again in the ADIOS2 backend since the benefits of keeping both enums separate were less than I'd thought in the beginning. I kept some of the refactoring, putting some code in private headers away from public includes. |
franzpoeschel
force-pushed
the
fix-adios2-char-datatype
branch
3 times, most recently
from
102b3ae to
fbfc490
Compare
franzpoeschel
commented
Jun 2, 2022
franzpoeschel
force-pushed
the
fix-adios2-char-datatype
branch
from
fbfc490 to
f738344
Compare
ax3l
reviewed
Jun 17, 2022
ax3l
reviewed
Jun 17, 2022
| } | ||
| #if defined(__INTEL_COMPILER) | ||
| /* | ||
| * ICPC has trouble with if constexpr, thinking that return statements are |
Member
There was a problem hiding this comment.
I think NVCC 11.0 has a similar issue, potentially check and add a similar suppression?
Around 11.5+, the false warning is solved.
ax3l
reviewed
Jun 17, 2022
| } | ||
| #if defined(__INTEL_COMPILER) | ||
| /* | ||
| * ICPC has trouble with if constexpr, thinking that return statements are |
ax3l
approved these changes
Jun 17, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
ADIOS2 now distinguishes between
char,signed charandunsigned char. Especially BP5 is unforgiving about this.Until now, the ADIOS2 backend uses the public
openPMD::Datatypeenum, which has already caused some issues in the past. This PR introduces a new internal enumADIOS2Datatypewhich is used to fully support ADIOS2 datatypes, especially char types. It is purely internal so we can change it as we please without breaking the API.Close #1273
Tests will mainly come in #1218 where BP5 will be a default backend for tests