axom icon indicating copy to clipboard operation
axom copied to clipboard

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

Open agcapps opened this issue 5 years ago • 4 comments

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 avatar Sep 17 '19 15:09 agcapps

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.

agcapps avatar Sep 17 '19 15:09 agcapps

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.

nselliott avatar Sep 19 '19 22:09 nselliott

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 avatar Nov 05 '21 23:11 nselliott

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.

nselliott avatar Nov 24 '21 22:11 nselliott

@nselliott make new issue to address documentation only.

rhornung67 avatar Jan 30 '23 22:01 rhornung67

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

nselliott avatar Feb 02 '23 19:02 nselliott