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

Using Maps/Dictionaries for attributes corresponding to axisLabels #1447

Open
DerNils-git opened this issue May 19, 2023 · 1 comment
Open
Assignees

Comments

@DerNils-git
Copy link
Contributor

Regarding the openPMD-standard attributes corresponding to specific axes (e.g. gridSpacing, globalGridOffset) of a mesh should be ordered in the same way as the axisLabels in their array. Currently this is just a suggestion to implementors which totally makes sense.

Guaranteeing that axisLabels match axis specific attributes is possible by storing these attributes e.g. in a C++ std::map<std::sting,double>. That makes it impossible to mess up axis specific attributes and axis labels.

Currently the implementor has to make sure that the order is correct which does not guarantee the correct order such that the following implementation mistakes are possible

openPMD::Mesh mesh{<SOME/MESH>};

std::vector<std::string> axisLabels = {"x","y","z"};
mesh.setAxisLabels(axisLabels);
std::vector<double> gridSpacing = {dx,dz,dy};
mesh.setGridSpacing(gridSpacing);

A safer way would be

openPMD::Mesh mesh{<SOME/MESH>};

std::vector<std::string> axisLabels = {"x","z","y"};
mesh.setAxisLabels(axisLabels);
std::map<std::string, double> gridSpacing = {{"x",dx},{"y",dy},{"z",dz}};
mesh.setGridSpacing(gridSpacing);

In this implementation additional safeguards could be added within openPMD such that attributes can only be added for axes that are already stored in axisLabels.

Possible Problem:
I am not sure how well a python dictionary interacts with a C++ std::map. This might be an issue in the python API of openPMD.

@franzpoeschel
Copy link
Contributor

At first glance, this sounds like a sensible and non-intrusive addition to the API. Will have a look

@franzpoeschel franzpoeschel self-assigned this May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants