GEOS-ESM / MAPL

MAPL is a foundation layer of the GEOS architecture, whose original purpose is to supplement the Earth System Modeling Framework (ESMF)
https://geos-esm.github.io/MAPL/
Apache License 2.0
26 stars 17 forks source link

History attribute 'regrid_method' is 'bilinear' if no regridding done #1888

Closed lizziel closed 1 year ago

lizziel commented 1 year ago

I noticed that the 'regrid_method' attribute in the history output files has value 'bilinear' when I output at native run resolution. For example:

    float SpeciesConc_pFe(time, lev, nf, Ydim, Xdim) ;
            SpeciesConc_pFe:_FillValue = 1.e+15f ;
            SpeciesConc_pFe:add_offset = 0.f ;
            SpeciesConc_pFe:coordinates = "lons lats" ;
            SpeciesConc_pFe:fmissing_value = 1.e+15f ;
            SpeciesConc_pFe:grid_mapping = "cubed_sphere" ;
            SpeciesConc_pFe:long_name = "Dry mixing ratio of species for pFe" ;
            SpeciesConc_pFe:missing_value = 1.e+15f ;
            SpeciesConc_pFe:regrid_method = "bilinear" ;
            SpeciesConc_pFe:scale_factor = 1.f ;
            SpeciesConc_pFe:standard_name = "Dry mixing ratio of species for pFe" ;
            SpeciesConc_pFe:units = "mol mol-1 dry" ;
            SpeciesConc_pFe:valid_range = -1.e+15f, 1.e+15f ;
            SpeciesConc_pFe:vmax = 1.e+15f ;
            SpeciesConc_pFe:vmin = -1.e+15f ;

This is confusing since no regridding is done. Could this be updated?

mathomp4 commented 1 year ago

Huh. Never noticed that. Obviously, I'll add @bena-nasa to this issue. (and cc @tclune)

I have some fear that we might need to have some regrid_method there just for file specification reasons on our end. I wonder if we might do something like:

            SpeciesConc_pFe:regrid_method = "native" ;

or maybe:

            SpeciesConc_pFe:regrid_method = "none" ;

or even:

            SpeciesConc_pFe:regrid_method = "identity" ;

I might leave it to @tclune to consider what might be the best way to say "This field was not regridded" with a regrid_method attribute.

@lizziel Would GCHP be open to something like this? And any preferences?

mathomp4 commented 1 year ago

Note: I think we are good with the current ADAS Cubed-Sphere collections in production at the moment (assuming I found the right output, might ping @rlucches to be sure) as it used a MAPL before this feature was added.

But this is probably something @rtodling would like in the new 5.30 ADAS as it is a confusing attribute.

lizziel commented 1 year ago

Yes, this solution would work for us. I brought this issue up at last week's Harvard-MIT-GMAO MAPL meeting and I remember Tom saying it should be "identity".

rlucches commented 1 year ago

Matt,

I am a bit confused about this. Our most recent production systems (GEOS-IT and FPP) specify in HISTORY a regrid_method setting of 'BILINEAR_MONOTONIC', which I think was intended to fix the round-off errors such as slight negative values for fields that should never be negative.

ctm_tavg_1hr_glo_C180x180x6_v72.regrid_method: 'BILINEAR_MONOTONIC' ,

This includes collections that we write out on the native cubed-sphere grid. The NetCDF files produced by these runs do not have a regrid_method metadata value. I am not sure if or how this relates to the issue that has been raised.

Thanks, Rob

On Mon, Dec 19, 2022 at 12:39:43PM -0800, Matthew Thompson wrote:

Note: I think we are good with the current ADAS Cubed-Sphere collections in production at the moment (assuming I found the right output, might ping @.*** to be sure) as it used a MAPL before this feature was added.

But this is probably something @.*** would like in the new 5.30 ADAS as it is a confusing attribute.

— Reply to this email directly, [3]view it on GitHub, or [4]unsubscribe. You are receiving this because you were mentioned. Message ID: @.***>

References

  1. https://gcc02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Frlucches&data=05%7C01%7Crobert.a.lucchesi%40nasa.gov%7C8fb34eb91c764b7fb83708dae2012a51%7C7005d45845be48ae8140d43da96dd17b%7C0%7C0%7C638070791874813495%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=i%2BpKyDwVnf0KzT%2F1u%2Bgg1eGbS243BzY4C%2B1ZaHhv%2BS4%3D&reserved=0
  2. https://gcc02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Frtodling&data=05%7C01%7Crobert.a.lucchesi%40nasa.gov%7C8fb34eb91c764b7fb83708dae2012a51%7C7005d45845be48ae8140d43da96dd17b%7C0%7C0%7C638070791874813495%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=qplC9XCwUTYLpbw9UpsvlQNQV38uWrCOOMqPH%2FZf7%2F0%3D&reserved=0
  3. https://gcc02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FGEOS-ESM%2FMAPL%2Fissues%2F1888%23issuecomment-1358297332&data=05%7C01%7Crobert.a.lucchesi%40nasa.gov%7C8fb34eb91c764b7fb83708dae2012a51%7C7005d45845be48ae8140d43da96dd17b%7C0%7C0%7C638070791874813495%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=dQoOSIi53J26Da5aBQuR%2FUK2b%2BI6K0AZJsDcjjyXql4%3D&reserved=0
  4. https://gcc02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAORRJXGFROS7NFSL36BTLJDWODBY7ANCNFSM6AAAAAATDYBBUQ&data=05%7C01%7Crobert.a.lucchesi%40nasa.gov%7C8fb34eb91c764b7fb83708dae2012a51%7C7005d45845be48ae8140d43da96dd17b%7C0%7C0%7C638070791874813495%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=WxO8LcLkWXOUIaRClBg6RowXz3smBqYTHZnkl1uMO3s%3D&reserved=0

--


Rob Lucchesi SSAI @.*** Global Modeling and Assimilation Office (GMAO) Operational Software Group (301) 614-6182 Code 610.1 NASA-Goddard Space Flight Center
Greenbelt, MD 20771

mathomp4 commented 1 year ago

@rlucches No. This is something I don't think you've seen yet in operations. In recent versions of MAPL, we added an attribute in the netCDF to detail what the type of regridding done was. The issue is that in native cubed-sphere output, we are saying it was regridded "bilinear" even when nothing was done. This is solely due to the fact that the default regridding method is bilinear, so we sort of always fill that in. We don't actually regrid cubed-sphere output, but the attribute mistakenly still has the default.

bena-nasa commented 1 year ago

@tclune @lizziel @mathomp4 Why did we even add this to the output in the first place? I can't see a practical reason to include it. If you don't like the method used, it is not as though you can do anything with this other than just not use it. Maybe we should get rid of it. For that matter why don't we add whether the variable was time averaged as an attribute, or if it was vertically regridded as an attribute, it just seems like if we are adding this, why not add all that stuff too, that is just as relevant for how it was processed for output... why does the Regridding method get an attribute and not those. Those seem just as relevant for making decisions but those are not encoded in the file metadata. Why does this get special treatment?

bena-nasa commented 1 year ago

@lizziel @mathomp4 @tclune If we are going to have this at all. We really should change the name to horizontal_regrid_method. Just plain regrid_method is rather disingenuous, what about the vertical if that was done?

tclune commented 1 year ago

I agree that if we keep it we need a more specific name. (Happy with @bena-nasa's suggestion.)

Question to @bena-nasa: Do we require users to specify a (horz) regrid method for each collection? I'm worried that with MAPL3 doing the regridding behind the scenes that History won't be able to detect any default regridding that MAPL uses.

(And TBD still is how to allow History to specify these regrid methods in the import spec.)

bena-nasa commented 1 year ago

Well, the way it works now, is that if History detects a grid_label, it passes the output_grid to my griddedIO class (each collection has an instantiation of this class) and you can also pass the regrid method to this API which if not passed defaults to "BILINEAR".

So we do not require the user to specify either an output grid or method for the collection. In that case, no output grid is passed to the griddedIO class, the input_grid is pulled from the input bundle, the output_grid is set to the input_grid and I guess the Regridding layer produces an identity regridderand the regrid_method is irrelevant in this case.

If they did specify an output grid in the collection that is delivered to the griddedIO class which obviously trigger something other than an identity regridder with the user requested horizontal method.

mathomp4 commented 1 year ago

Also, one thought from @bena-nasa, should we even have a regrid_method (or horizontal_regrid_method) metadata if it's identity?

I mean, in a way if no regridding was done, regrid_method: identity is almost superfluous. Not sure.

lizziel commented 1 year ago

I agree with @mathomp4 that having regrid_method if no regridding is done is superfluous, and also with @bena-nasa that it doesn't make sense that there is horizontal regridding attribute but not others, such as vertical regridding. I also wonder why regrid_method is at the variable level. Since a collection only has one regrid method, if any, shouldn't this be global metadata?

mathomp4 commented 1 year ago

I agree with @mathomp4 that having regrid_method if no regridding is done is superfluous, and also with @bena-nasa that it doesn't make sense that there is horizontal regridding attribute but not others, such as vertical regridding. I also wonder why regrid_method is at the variable level. Since a collection only has one regrid method, if any, shouldn't this be global metadata?

@lizziel While #1922 does fix this, it will do identity. We'll do the "no regrid_method if identity" with MAPL3 since we don't want to change MAPL output in MAPL2 (as someone might be depending on it).

lizziel commented 1 year ago

Sounds good.