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.

Sign up for GitHub

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

Jump to bottom

Random-access for variable-based encoding (i.e. use SetStepSelection for ADIOS2 steps) #1706

Open
wants to merge 31 commits into
base: dev
Choose a base branch
from
Open

Random-access for variable-based encoding (i.e. use SetStepSelection for ADIOS2 steps) #1706

wants to merge 31 commits into from

Conversation

franzpoeschel
Copy link
Contributor

@franzpoeschel franzpoeschel commented Dec 16, 2024 •
edited
Loading

TODO:

  • MPI-aware preparsing
  • Maybe through out MPI-awareness of preparsing again
  • Edge case: What if a particle species completely vanishes after some iteration, making it appear as an empty, i.e. constant component -> unsupported case, backend will throw
  • Better testing
  • Documentation, examples
  • Replace group-based encoding with variable-based encoding more thoroughly for ADIOS2 (maybe follow-up)
  • Can we make variable-based encoding the default output for ADIOS2? (maybe follow-up for easier reversibility)
  • Check if the engine supports this mode ---> would include HDF5 engine, but it has no first-class support, so setting group-based encoding manually is fine
  • Check if the datasets have the same number of steps as there are snapshots
  • Add sth like "UnselectStepSelection" for reading /rankTable
  • follow-up: maybe pre-parse the entire thing in linear encoding and only then switch to random-access?
  • follow-up: Preparsing also in READ_LINEAR (i.e. eliminate duplicate Iterations better than now)
  • For defaulting v-encoding: Do attributes such as dt or time make sense here at all? ---> Need to preparse those, too. Check all modifiable attributes upon first READ_LINEAR open. --> follow-up, print warning for now
  • Maybe use openPMD 2.0 to introduce a more breaking default for v-based encoding in ADIOS2? --->follow-up

@franzpoeschel franzpoeschel added backend: ADIOS2 api: new additions to the API labels Dec 16, 2024
@franzpoeschel franzpoeschel force-pushed the random-access-steps branch 2 times, most recently from 4f7284b to 585b084 Compare December 19, 2024 13:35
@franzpoeschel franzpoeschel force-pushed the random-access-steps branch 5 times, most recently from 238fcd1 to 688ffa9 Compare January 15, 2025 14:23
@franzpoeschel
Add preparseSnapshots
c455ed3
@franzpoeschel
Serial ADIOS2 implementation for readAttributeAllsteps
d0351b7
@franzpoeschel
Feature-complete, untested
ea55a8b
@franzpoeschel
Fixes
3830358
@franzpoeschel
wip: parallel impl. for readAttributeAllsteps
d51765f 
huh this works??
@franzpoeschel
Simplify, cleanup
b957879
@franzpoeschel
wip testing
77591eb
@franzpoeschel
CI fixes
a7c7b2b
@franzpoeschel
Fix test
1af6e3a
@franzpoeschel
Add MPI_CHECK
5c4ab83
@franzpoeschel
Stricter warning about group-based encoding in BP5
f4f1fce
@franzpoeschel
variableBased as default in ADIOS2
aec0f1c
@franzpoeschel
Add error check for non-homogeneous datasets
c9fcf23
@franzpoeschel
Reset steps before reading the rankTable
2ac1396
@franzpoeschel
Use writeIterations() in MPI Benchmark
f845022
@franzpoeschel
Use group tables by default
b4f4439
@franzpoeschel
Add iterate_nonstreaming_series test to parallel tests
dda0c14 
This covers the use case that the snapshot attribute has more than just
one entry.
@franzpoeschel
Remove std::cout calls
2445c95
@franzpoeschel
wip: use variable-based encoding in tests
ac59be6
@franzpoeschel
CI fixes
25c9594
@franzpoeschel
Fix Windows testing
ecf7039
franzpoeschel
franzpoeschel commented Feb 13, 2025
View reviewed changes
include/openPMD/IO/ADIOS/ADIOS2IOHandler.hpp
@@ -854,6 +861,11 @@ class ADIOS2IOHandler : public AbstractIOHandler
return "ADIOS2";
}

bool fullSupportForVariableBasedEncoding() const override
{
return openPMD_HAS_ADIOS_2_9;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe exchange for constant true once the minimum supported version of ADIOS2 has been raised to 2.9

franzpoeschel
franzpoeschel commented Feb 13, 2025
View reviewed changes
test/ParallelIOTest.cpp
read_variablebased_randomaccess::read_variablebased_randomaccess();
}

TEST_CASE("iterate_nonstreaming_series", "[serial][adios2]")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test is also modified by #1646, this will lead to merge conflicts.

github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 13, 2025
View reviewed changes
include/openPMD/Iteration.hpp Dismissed Show dismissed Hide dismissed
include/openPMD/Iteration.hpp Dismissed Show dismissed Hide dismissed
This was referenced Feb 13, 2025
Open
Open
github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 14, 2025
View reviewed changes
test/CoreTest.cpp
@@ -1499,6 +1500,11 @@
#endif
}

TEST_CASE("automatic_variable_encoding", "[adios2]")

Check notice

Code scanning / CodeQL

Unused static function Note test

Static function C_A_T_C_H_T_E_S_T_54 is unreachable (
autoRegistrar55
Loading
must be removed at the same time)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
api: new additions to the API backend: ADIOS2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@franzpoeschel