Make `Diag` structure within FV3GFS_io.F90 public? about shield_physics

[spencerkclark]

Per our discussion today, I went ahead and made a PR to make this change (#30).

[lharris4]

Hi, Spencer. I am in favor of this change. Given the nature of SHiELD I think it is OK to just make the data structure a public object rather than go to the work of creating an API of getters and setters. (We could place a label on it: ``No user serviceable parts inside''.)

I will defer to @bensonr @laurenchilutti @linjiongzhou as to whether this is the best approach but I am in favor of it.

Thanks
Lucas

[bensonr]

@spencerkclark - I haven't looked at the code in quite a few years, but the FV3GFS_io.F90 Diag type should be aliased to the IPD_Diag and available at the atmos_model_mod (atmos_model.F90) level of the model.

[spencerkclark]

Thanks @bensonr. This is indeed the case in our old fork of FV3GFS (which is how we've set things up there), but in SHiELD I think FV3GFS_io_mod.Diag is (somewhat confusingly) separate from atmos_model_mod.IPD_Diag. atmos_model_mod.IPD_Diag is populated in physics_diag_layer.diag_populate, while FV3GFS_io_mod.Diag is populated in FV3GFS_io_mod.gfdl_diag_register, and these two data structures are not populated with the same data.

FV3GFS_io_mod.Diag is what is actually used in writing out diagnostics in FV3GFS_io_mod.gfdl_diag_output. Conversely, while it gets populated in the IPD_initialize routine, I'm not sure if atmos_model_mod.IPD_Diag ever ends up being used for anything meaningful within SHiELD.

I'm guessing there is a story behind this that I am not familiar with.

[linjiongzhou]

I'm just a user of these functions, so I'll leave it to Rusty and Lauren to determine the most suitable approach.

Linjiong

[lharris4]

It's a little challenging to tease out the abstractions here. This IPD_Diag of IPD_Diag_type holds a lot of metadata for each variable, each of which then includes a pointer to data within Diag of intdiag_type which is an alias of GFS_diag_type. The GFS_diag_type does not hold metadata, just the individual datasets, whereas IPD_Diag is a generic container which can hold a number of different datasets.

In any case, I believe IPD_Diags are just for I/O and do not control the model state itself, so reading it should be safe as long (as you said) you don't read from a variable which is being accumulated.

All of this should be pretty similar between FV3GFS and the most recent version of SHiELD since the IPD layer has not been heavily modified over the years.

If you wish to actually override data, such as in the surface datasets, that is a different story. I think sfcprop_type => GFS_sfcprop_type is what you want to modify.

[spencerkclark]

This IPD_Diag of IPD_Diag_type holds a lot of metadata for each variable, each of which then includes a pointer to data within Diag of intdiag_type which is an alias of GFS_diag_type. The GFS_diag_type does not hold metadata, just the individual datasets, whereas IPD_Diag is a generic container which can hold a number of different datasets.

Yes, I am familiar with how all of this works. The point I am bringing up is that in SHiELD there is another (non-identical) version of this metadata infrastructure (including some type definitions) in FV3GFS_io.F90 (Diag <-> IPD_Diag; gfdl_diag_type <-> IPD_diag_type; Diag is populated here <-> IPD_Diag is populated here). There is overlap between the diagnostics that are defined in FV3GFS_io.F90 and those that are defined in GFS_diagnostics.F90, but they are not the same, and the source of truth for SHiELD is in Diag in FV3GFS_io.F90, and not IPD_Diag.

If you wish to actually override data, such as in the surface datasets, that is a different story. I think sfcprop_type => GFS_sfcprop_type is what you want to modify.

Yes, this is the same as in FV3GFS, and (among other ways) is how we modify variables in the physics, which is enabled by NOAA-GFDL/atmos_drivers#26. We indeed only care about "getting" data in the case of these diagnostics fields.

[lharris4]

Glad we can move forward on this.