-
Notifications
You must be signed in to change notification settings - Fork 51
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
ADIOS2 schema 2022_07_26, based on ADIOS2 modifiable attributes #1310
Conversation
7044d3d
to
d089c1d
Compare
70b0356
to
4ae06b9
Compare
07af1b6
to
22fb4dc
Compare
22fb4dc
to
b3ac421
Compare
b3ac421
to
526e894
Compare
526e894
to
539304c
Compare
bd748ad
to
220c203
Compare
9d7bb3a
to
ab89d27
Compare
ab89d27
to
f09d86e
Compare
9d06458
to
07dbb48
Compare
07dbb48
to
8e8bdc9
Compare
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.
This is great, thank you!
I added small inline comments for suggestions.
There is a small conflict in docs that needs to be resolved now, from merge of other PRs.
Once this leaves experimental state in future releases, we should remember to document this in FORMAT_ADIOS.md
of openPMD-standard: https://github.com/openPMD/openPMD-standard/blob/upcoming-2.0.0/FORMAT_ADIOS.md
/* | ||
* ADIOS2 v2.8 brings mode::ReadRandomAccess | ||
*/ | ||
#define HAS_ADIOS_2_8 (ADIOS2_VERSION_MAJOR * 100 + ADIOS2_VERSION_MINOR >= 208) | ||
/* | ||
* ADIOS2 v2.9 brings modifiable attributes (technically already in v2.8, but | ||
* there are too many bugs, so we only support it beginning with v2.9). | ||
* Group table feature requires ADIOS2 v2.9. | ||
*/ | ||
#define HAS_ADIOS_2_9 (ADIOS2_VERSION_MAJOR * 100 + ADIOS2_VERSION_MINOR >= 209) |
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 wonder if these should be runtime functions instead?
Anyway, the way you use them here macros make sense.
One thing to consider: this is a header file. You might want to undefine the two macros at the end of it, especially with the generic name.
Since you use the macros around a few locations (e.g., also in tests), maybe provide them in a helper header file?
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 wonder if these should be runtime functions instead?
We could maybe add something like auto Series::backendInfo() const -> std::map<std::string, std::string>
analogous to auto Series::backendName() const -> std::string
for exposing this information to users? Would be outside of the scope of this PR though.
One thing to consider: this is a header file. You might want to undefine the two macros at the end of it, especially with the generic name.
The macros are needed in the including *.cpp
s. Since these headers are internal and not exposed to user code, this is probably not so critical, but I will rename them to OPENPMD_HAS_ADIOS...
.
Since you use the macros around a few locations (e.g., also in tests), maybe provide them in a helper header file?
That's why I moved them to ADIOS2Auxiliary.hpp
in the first place :D But given that I will be renaming the macros, creating their own file is fine, too.
07a32cd
to
d84898f
Compare
Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>
d84898f
to
c3cfa09
Compare
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.
🚀 ✨
Based on (and to a large part compatible with) the old ADIOS2 schema 0, this PR removes schema 2021, using instead the
allowModification
parameter of theDefineAttribute()
call in ADIOS2 to bring a better support for ADIOS2 steps (which was the main motivation for schema 2021).Problem of schema 0: It was impossible to associate groups to single steps, making it unusable for variable-based iteration encoding. In schema 0, the group hierarchy was restored at read time indirectly by inquiring attributes and variables. Since attributes cannot be deleted in ADIOS2, this makes it impossible to delete a group once defined.
The new schema (2022) introduces a meta table for tracking active groups in the hierarchy, see an example dataset created by the
variableBasedSeries
test:For each IO step
i
and active path<p>
, the value of the modifable attribute__openPMD_groups/<p>
is then set asi
in that step.(Note that the simpler alternative of using a boolean "active" flag
openPMD_group_is_active/<p>
=true
or similar does not work in parallel contexts. Using the step index has the advantage that the flag needs not be re-set.)The
LIST_PATHS
IO task can then be implemented by using only the group table. A path exists if:Using a table in this form allows for an algorithmically quick lookup via prefix search in a sorted map, and also visually declutters the metadata by putting the table in one block as seen in the above output of
bpls -alt
.Such tricks are necessary only for paths, not for datasets, since ADIOS2 variables (i.e. datasets) are clearly associated with IO steps.
Drawback that we accept with this design: Unlike groups, an attribute once written can only be modified
(not yet implemented), but not deleted. We need to expose theallowModification
tag somehow though to enable mutable user-defined attributes (see TODOs).TODO
__is_boolean__
to__openPMD_internal/is_boolean
. Apart from this, the 2022 schema is forward- and backward-compatible to the old one. So, we should discuss if we want to stick with the renaming, or just keep the old name.adios2.use_group_table = false
.