search

NOAA-CEFI-Regional-Ocean-Modeling / ocean_BGC

3 stars 4 forks source link

Cleanup idea: eliminate unnecessary vardesc #15

Open andrew-c-ross opened 3 months ago

andrew-c-ross commented 3 months ago

When diagnostics are registered, first a vardesc type is populated with some metadata, and then some items in the type are used to register the diagnostic. For example, https://github.com/NOAA-CEFI-Regional-Ocean-Modeling/ocean_BGC/blob/bfa02118f1bda46cc230d18074401e6d782a8e74/generic_tracers/generic_COBALT.F90#L1906-L1908

I don't think the vardesc structure has any use and it could be replaced with directly passing the metadata to register_diag_field as is done in MOM6. For example,

 CS%id_masscello = register_diag_field('ocean_model', 'masscello', diag%axesTL, &
      Time, 'Mass per unit area of liquid ocean grid cell', 'kg m-2', conversion=GV%H_to_kg_m2, &
      standard_name='sea_water_mass_per_unit_area', v_extensive=.true.)

https://github.com/NOAA-CEFI-Regional-Ocean-Modeling/MOM6/blob/c1367a9b8171e74816bdf210f32876bb75edc5f9/src/diagnostics/MOM_diagnostics.F90#L1623-L1625

This would eliminate almost 800 lines of code from COBALT alone, and also make it easier to declare new diagnostics since it won't be necessary to specify the unused items in vardesc (hor_grid, z_grid, t_grid, mem_size).

andrew-c-ross commented 3 months ago

I will note that MOM_io.F90 uses a similar vardesc type, and all of the extra tracers in MOM6/src/tracer also do some things with it. So if there was some sort of intention to merge this functionality into the generic tracers we would not want to make the proposed deletion.

yichengt900 commented 3 months ago

@andrew-c-ross, I see your point. I guess the original idea behind the vardesc type is for it to be reused/called by several different subroutines to provide all the necessary metadata information. It might be a good idea to move this function to generic_tracer_utils and make it public. Currently, it is only used by the register_diag_field subroutine, but this may change in the future (e.g., a new runtime parameter output subroutine might use it). Perhaps we can address #14 first and revisit this one later.

yichengt900 commented 2 months ago

This issue is partially resolved in #43 by moving all the diagnostic registration calls to a separate module. The next step could be to consider implementing Kelly's approach to automatically generate this module.