WIP: Add OpenPMD support #1050
Open
WIP: Add OpenPMD support #1050
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
pgrete
commented
Apr 12, 2024
src/outputs/openpmd.cpp
Outdated
Comment on lines
272
to
275
| for (auto &pmb : pm->block_list) { | ||
| // TODO(pgrete) check if we should skip the suffix for level 0 | ||
| const auto level = pmb->loc.level() - pm->GetRootLevel(); | ||
| const std::string &mesh_record_name = var_name + "_lvl" + std::to_string(level); |
pgrete
commented
Apr 12, 2024
src/outputs/restart_opmd.cpp
Outdated
Comment on lines
120
to
125
| for (auto &pmb : pm->block_list) { | ||
| // TODO(pgrete) check if we should skip the suffix for level 0 | ||
| // TODO(pgrete) ask LR why this is not mirrored from writing | ||
| // const auto level = pmb->loc.level() - pm->GetRootLevel(); | ||
| const auto level = pmb->loc.level(); | ||
| const std::string &mesh_record_name = var_name + "_lvl" + std::to_string(level); |
Yurlungur
approved these changes
Apr 25, 2024
There was a problem hiding this comment.
Many comments below. However, because this was such a big lift, I am of the opinion that we should merge even a not feature complete or not perfect contribution so that it doesn't start to drift away from develop. And the full feature complete capability can be created via later PRs. So I'm approving.
| fill_derived = false # whether to fill one-copy test vars | ||
|
|
||
| <parthenon/output1> | ||
| file_type = openpmd |
There was a problem hiding this comment.
Does openpmd always output everything?
There was a problem hiding this comment.
It currently mimics restart output (with support for additional explicit variables), but this doesn't need to remain that way.
| @@ -118,6 +118,12 @@ class Params { | |||
| return it->second; | |||
| } | |||
|
|
|||
| const Mutability &GetMutability(const std::string &key) const { | |||
Comment on lines
+360
to
+361
| // TODO(reviewers) Why was the loop originally there? Does the direct Get causes | ||
| // issue? |
There was a problem hiding this comment.
I'm not sure---it may have just been crazy code that no one tried to change.
| @@ -241,6 +243,45 @@ std::vector<int> ComputeIDsAndFlags(Mesh *pm) { | |||
| }); | |||
| } | |||
|
|
|||
| template <typename T> | |||
| std::vector<T> FlattendedLocalToGlobal(Mesh *pm, const std::vector<T> &data_local) { | |||
There was a problem hiding this comment.
I don't understand what this function does. Is it actually doing an MPI all-to-all to build up a global data vector? Is this something we ever want to do?
| it.setAttribute("blub", host_vec); | ||
|
|
||
| for (const auto &[key, pkg] : pm->packages.AllPackages()) { | ||
| // WriteAllParams<bool>(pkg, &it); // check why this (vector of bool) doesn't work |
There was a problem hiding this comment.
Is it just std::vector<bool> that doesn't work? That's probably because std::vector<bool> has a particular template specialization in the stl that is weirdly incompatible with generic code. Probably need to write a pointer-based array of bool instead.
Comment on lines
+316
to
+318
| // TODO(pgrete) check var name standard compatiblity | ||
| // e.g., description: names of records and their components are only allowed to contain | ||
| // the characters a-Z, the numbers 0-9 and the underscore _ |
There was a problem hiding this comment.
This is broken by PR #1043
Comment on lines
+344
to
+348
| // TODO(pgrete) adjust for single prec output | ||
| // openPMD::Datatype dtype = openPMD::determineDatatype<Real>(); | ||
| using OutT = | ||
| Real; // typename std::conditional<WRITE_SINGLE_PRECISION, float, Real>::type; | ||
| std::vector<OutT> tmp_data(var_size_max * num_blocks_local); |
Comment on lines
+474
to
+489
| for (int t = 0; t < Nt; ++t) { | ||
| for (int u = 0; u < Nu; ++u) { | ||
| for (int v = 0; v < Nv; ++v) { | ||
| const auto [record_name, comp_name] = | ||
| OpenPMDUtils::GetMeshRecordAndComponentNames(vinfo, comp_idx, level); | ||
| auto mesh_comp = it.meshes[record_name][comp_name]; | ||
|
|
||
| const auto comp_offset = tmp_offset; | ||
| for (int k = kb.s; k <= kb.e; ++k) { | ||
| for (int j = jb.s; j <= jb.e; ++j) { | ||
| for (int i = ib.s; i <= ib.e; ++i) { | ||
| tmp_data[tmp_offset] = static_cast<OutT>(out_var_h(t, u, v, k, j, i)); | ||
| tmp_offset++; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Feels like this should be possible to merge with the higher-order functions in utils
BenWibking
reviewed
May 17, 2024
| # Given that the shapes are guaranteed to match (follow the check above) | ||
| # we can load chunks from both files. | ||
| # Note that we have to go over chunks as data might be sparse on disk so | ||
| # loading the entire record will contain gargabe in sparse places. |
|
In terms of packaging the OpenPMD dependency, is the user expected to install OpenPMD themselves to get the Python package? If I don't have OpenPMD installed externally, it uses If I do have openpmd-api installed externally, it doesn't complain, but it still uses the built-in openPMD module for the C++ build, which (I think) could lead to an inconsistency. |
|
Also, we now have two very confusingly named CMake options for I/O: |
|
I am getting a CMake configuration error when trying this:
Edit: ok, it turns out this is a weird Conda environment issue. You have to explicitly request MPI support when installing $ conda install -c conda-forge "openpmd-api=*=mpi_openmpi*"TL;DR: I think this is a good reason to ensure that the internal OpenPMD build also installs the Python module. |
|
I am getting regression test failures on my Mac laptop: They all have mysterious errors like this: |
|
I propose installing the OpenPMD python package from the submodule using, e.g.: |
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.
PR Summary
PR Checklist