E3SM icon indicating copy to clipboard operation
E3SM copied to clipboard

Feature Request: Support horiz_remap_file for np4/gll -> dst grid

Open jsbamboo opened this issue 9 months ago • 10 comments

Feature Request: Support horiz_remap_file for np4/gll -> dst grid (follow-up to #7257)

Motivation

The horiz_remap_file functionality currently works for pg2 -> dst grids but fails for np4/gll -> the same dst grid.

From discussion in #7257, this is currently disabled by design.

Justification

It would be very helpful to enable statistical diagnostics involving dycore variables, where access to native-grid or small-region output is key.

jsbamboo avatar Apr 17 '25 17:04 jsbamboo

For context, the issue is that we want to remap twice: once from dynamics to physics_gll, and once from physics_gll to the tgt grid. IO does not allow this. However, there is no real motivation not to allow it. We would have to

  • bring back the "combo" remapper, to pipe remappers together
  • make IO create the horiz remapper as a combo remapper (if needed)

bartgol avatar Apr 17 '25 22:04 bartgol

thanks for the context! ^^

jsbamboo avatar Apr 18 '25 00:04 jsbamboo

I think there may be some confusion here. @jsbamboo is wanting to output remapped dynamics grid data, and has a np4/gll map file to the dst grid. @bartgol seems to be implying that we should do this with two map files (dyn->pg2->dst), and thus we need a combo remapper. But will will obtain more accurate results and dont need new functionality if EAMxx just maps dyn->dst with the supplied map file.

From https://github.com/E3SM-Project/E3SM/issues/7257, it seems the issue is if one is running with the pg2 physics grid, but specifies an output file of only np4 data, with the correct np4 mapping file, the code crashes. These seems like a bug? I'll go ahead and guess that this bug could be in the code that validates the mapping file. That code might assume all remapped data is pg2, and is not aware that users might be requesting remapping of np4 data (as outputting np4 data is uncommon and usually only done for development reasons). @jsbamboo might be the first person to ever try to output remapped np4 data from a pg2 simulation.

mt5555 avatar Apr 18 '25 15:04 mt5555

I'm a bit confused... I initially thought @bartgol meant that we need a path like dyn -> physics_gll (which corresponds to the mapping file's np4 grid?), followed by -> dst. Though I still don't quite understand why the first step dyn -> physics_gll is always required..(this post seems to help me better understand that step: https://github.com/E3SM-Project/scream/issues/1298)

The issue raised in the second point by @mt5555 was also my initial guess — that the correct src grid (np4) couldn't be found. But since the request is clearly for Dynamics fields and the I/O grid name is explicitly set to physics GLL, I assumed the correct grid should be able to be determined?

Thanks @mt5555 for helping clarify the need! I had searched the repo earlier but didn't find any YAML files for np4 -> whatever remapping, which is why I opened this issue

jsbamboo avatar Apr 18 '25 16:04 jsbamboo

I just commented based on what was in the yaml file shown here. It is true, however, that one could do just dynamics->tgt_grid, provided that a map file SE->tgt exists.

That said, I am not entirely sure our framework would work. I think the answer is "maybe?". We never used a map file in the context where the source grid is not "unique" (meaning that each dof GID is owned by a single rank). The SE grid is not unique, so it's a bit of uncharted territory... Either way, if this is what you wnat to do, then the map file should be np4->tgt regardless of whether physics runs on pg2 or gll points, since you are not remapping physics states, but dyn states.

@jsbamboo you could try running without the option IO Grid Name: Physics GLL in your yaml file, and see what happens.

Edit: I think our remapper would complain that the number of dofs in the src grid (SE, with nene6npnp "non-unique" columns) does not match the value n_a in the map file...

bartgol avatar Apr 18 '25 16:04 bartgol

Ah, yes, the SE grid would not currently work as a src grid, b/c of this check in the remapper construction:

  EKAT_REQUIRE_MSG (fine_grid->is_unique(),
      "Error! HorizInterpRemapperBase requires a unique fine grid.\n");

We can of course try to see if this requirement can be removed. I think the answer may be "yes", but the code there is a bit complex, so it may take some time to make sure we're doing it right...

bartgol avatar Apr 18 '25 17:04 bartgol

There must be code already to convert data from the np4 DG representation to the uniquepoints representation. It most likely is used when running np4 physics, and also when outputing dynanmics fields with "IO Grid Name: Physics GLL". So maybe the fix would be to do this transformation before calling the I/O remap code?

mt5555 avatar Apr 18 '25 17:04 mt5555

@bartgol Yes I have this test (without assigning IO Grid Name: Physics GLL in the yaml):

I also missed the inspiring error message in e3sm.log for this test ☹️:

7316 FAIL: 7317 fine_grid->type()==GridType::Point 7318 /p/lustre2/zhang73/GitTmp/WPRRMxx/components/eamxx/src/share/grid/remap/horiz_interp_remapper_base.cpp:27 7319 Error! Horizontal interpolatory remap only works on PointGrid grids. 7320 - fine grid name: Dynamics 7321 - fine_grid_type: SE

this test doesn't have the ERROR: scorpio_output - online remapping not supported with vertical and/or horizontal remapping from file message shown in the test with IO Grid Name: Physics GLL in the yaml

jsbamboo avatar Apr 18 '25 17:04 jsbamboo

There must be code already to convert data from the np4 DG representation to the uniquepoints representation. It most likely is used when running np4 physics, and also when outputing dynanmics fields with "IO Grid Name: Physics GLL". So maybe the fix would be to do this transformation before calling the I/O remap code?

That's what I meant with the "combo remapping" option. The Dyn->PhysGLL remap is done via a "remapper" (in eamxx lingo). Currently, IO does not allow to also use a coarsening remapper when the Dyn->PhysGLL one is active. My suggestion would be to remove this assumption from IO, and allow IO to build the Dyn->PhysGLL->CoarseGrid "combo remapper".

bartgol avatar Apr 18 '25 20:04 bartgol

Sorry, I completely missed that. Way too much abstraction of the "remap" operation for me :-)

mt5555 avatar Apr 18 '25 22:04 mt5555