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

variableBased Encoding should be limited to ADIOS #1481

Closed
guj opened this issue Jul 19, 2023 · 4 comments · Fixed by #1484
Closed

variableBased Encoding should be limited to ADIOS #1481

guj opened this issue Jul 19, 2023 · 4 comments · Fixed by #1484

Comments

@guj
Copy link
Contributor

guj commented Jul 19, 2023

Describe the bug
variableBased encoding with HDF5 produced error for not able to create (an existing) dataset.

To Reproduce
in WarpX, let drag use both h5 and v; e.g.
diag1.openpmd_encoding=v
diag1.openpmd_backend=h5

Expected behavior
HDF5 will throw error when written second step.

** comments **.
I think possible solution can be at setIterationEncoding(inputEncoding), it first check with I/O handler whether inputEncoding is allowed. If not, fall to a default one (groupBased)

@guj guj added the bug label Jul 19, 2023
@franzpoeschel
Copy link
Contributor

Thank you for opening an issue for this

I think possible solution can be at setIterationEncoding(inputEncoding), it first check with I/O handler whether inputEncoding is allowed. If not, fall to a default one (groupBased)

I think that we should recognize this situation and throw an error accordingly. It's simpler to do and does not do unexpected things. Also note this test: Variable-based encoding is supported for all backends as long as only one iteration is written. The intended use case is for datasets that don't need iterations and can hence forgo one layer in the file hierarchy.

@franzpoeschel franzpoeschel self-assigned this Jul 24, 2023
@guj
Copy link
Contributor Author

guj commented Jul 24, 2023

True, files with one iteration should always to allowed.
Then defaulting variable encoding in h5 to file encoding would cover it.

I am not sure whether it is serious enough to exit with error just because variable encoding in not supported.
If I were a client, I might prefer to continue with a supported encoding.

I think that we should recognize this situation and throw an error accordingly. It's simpler to do and does not do unexpected things. Also note this test: Variable-based encoding is supported for all backends as long as only one iteration is written. The intended use case is for datasets that don't need iterations and can hence forgo one layer in the file hierarchy.

@franzpoeschel
Copy link
Contributor

I am not sure whether it is serious enough to exit with error just because variable encoding in not supported.
If I were a client, I might prefer to continue with a supported encoding.

This situation means that a user configured openPMD in a way that implies data loss. We do have error::OperationUnsupportedInBackend for exactly those cases.

This situation can only be detected at the point where the user tries opening a second step, since we support writing a single step. At this point, it's too late to fix things since the first step is already written.

@guj
Copy link
Contributor Author

guj commented Jul 25, 2023

OK, sure.

This situation means that a user configured openPMD in a way that implies data loss. We do have error::OperationUnsupportedInBackend for exactly those cases.

This situation can only be detected at the point where the user tries opening a second step, since we support writing a single step. At this point, it's too late to fix things since the first step is already written.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants