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

Group::save then load round trip should give equivalent result #97

Closed
agcapps opened this issue Sep 17, 2019 · 6 comments
Closed

Group::save then load round trip should give equivalent result #97

agcapps opened this issue Sep 17, 2019 · 6 comments
Assignees
Labels
bug Something isn't working Reviewed Sidre Issues related to Axom's 'sidre' component

Comments

@agcapps
Copy link
Member

agcapps commented Sep 17, 2019

We currently guarantee equivalency when using the sidre_hdf5 protocol to save a Group then load it into another group.

  • Currently the tests for equivalency after load() all have if (protocol != "sidre_hdf5") {continue;}
  • Should other sidre_ protocols support equivalent save/load round trips?
  • What about other protocols that (for example) might not store enough information to distinguish int bit-width? Do we need a predicate in addition to Group::isEquivalent to test if groups are "pretty close"?
@agcapps agcapps added bug Something isn't working Sidre Issues related to Axom's 'sidre' component labels Sep 17, 2019
@agcapps
Copy link
Member Author

agcapps commented Sep 17, 2019

I tagged this as a bug because it's not a feature, but it is something we need to add tests for and more clearly document.

@nselliott
Copy link
Contributor

The test "save_load_all_protocols" does do a save/load equivalence test over all of the procotols. It does rig things a bit for the json protocols by being coded with pre-knowledge that they going to save plain-text int and float values into 64-bit types.

The other tests do skip protocols that aren't sidre_hdf5, so this issue is still worthwhile to take a look at those to decide where and how we should be testing the other protocols.

@rhornung67 rhornung67 added this to the FY22 Development milestone Oct 12, 2021
@nselliott nselliott self-assigned this Nov 5, 2021
@nselliott
Copy link
Contributor

Another case where save/load round trips do not produce equivalence is that some protocols (non sidre_*) do not preserve an empty sub-Group through a save/load operation.

@nselliott
Copy link
Contributor

I can't find anything in the docs that describes the meaning of the various protocols, so that is something that ought to be added. I don't think we can make it so that save/load round trips preserve identical data for every protocol in every situation, but we should document that, and round trips do work more broadly than just sidre_hdf5 which is the only one extensively tested.

@rhornung67
Copy link
Member

@nselliott make new issue to address documentation only.

@nselliott
Copy link
Contributor

Closing with new issue #1019 covering these topics more directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Reviewed Sidre Issues related to Axom's 'sidre' component
Projects
None yet
Development

No branches or pull requests

3 participants