pmb->loc.level() differences in writing and reading restart files

[pgrete]

For the OpenPMD output #1050 I need to create a logical mesh for each level.

For writing, I use https://github.com/parthenon-hpc-lab/parthenon/pull/1050/files#r1562678570

    for (auto &pmb : pm->block_list) {
      const auto level = pmb->loc.level() - pm->GetRootLevel();
      const std::string &mesh_record_name = var_name + "_lvl" + std::to_string(level);

This resulted in following variables in the output file following the standard (with the root level being 0)

$ bpls ../opmd.00001.bp
  double   /data/1/meshes/advected_lvl0  {4, 64, 64}
  double   /data/1/meshes/advected_lvl1  {8, 128, 128}
  double   /data/1/meshes/advected_lvl2  {16, 256, 256}

For reading I originally mirrored this, but eventually had to remove the - pm->GetRootLevel(); from calculating the level https://github.com/parthenon-hpc-lab/parthenon/pull/1050/files#r1562680579

  for (auto &pmb : pm->block_list) {
    // 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);

as on restart pmb->loc.level() returned already either 0, 1, or 2 but pm->GetRootLevel(); was 2 resulting in wrong level indices.

@lroberts36 any clue what's going on?

[lroberts36]

@pgrete: There is a bug in parthenon_hdf5.cpp. On line 94 it should be a call to 'GetRootLevel()' instead of getting the legacy root level I think. Sorry I missed that when switching over. This may necessitate an update of gold files though...

[pgrete]

Good catch. Any idea why this hasn't caused any major issues? Is restarting without knowing/setting the correct root_level possible/working?
This question is also tightly connected to: should we issue a quick bugfix or should I fix it along the pmd output?

[lroberts36]

@pgrete: I think we should put up a quick bugfix, which I have done in #1053.

BTW, my previous comment is incorrect since we are currently doing all output and restarts assuming the hdf5 files include legacy tree locations. We need to change this over at some point.