NOAA-CEFI-Regional-Ocean-Modeling / ocean_BGC

3 stars 4 forks source link

Eliminate repeated diagnostic ID checks and refactor send_diag section #23

Closed yichengt900 closed 2 months ago

yichengt900 commented 3 months ago

This PR addresses issue #14. Several code changes are included:

  1. Removed diagnostic ID check lines from the original generic_COBALT_update_from_source and generic_COBALT_update_from_bottom subroutines .
  2. Created a new file cobalt_glbl.F90 and moved phytoplankton, zooplankton, bacteria, and generic_cobalt data types as well as namelist here.
  3. Created a new file cobalt_send_diag.F90 and added a new subroutine cobalt_send_diagnostics to the cobalt_send_diag module to handle send_diag calls separately.
  4. Added a new subroutine cobalt_send_diagnostics_bottom to the cobalt_send_diag module to handle send_diag calls for bottom fluxes.
  5. Reduced the total number of lines in generic_COBALT.F90 from 16169 to 11484 (a 28.91% reduction).
  6. Regarding computational time, the average runtime before and after refactoring in 1d case is 11.50 seconds versus 11.60 seconds, representing an approximate 0.87% increase.
  7. The following namelist capabilities have been removed (mentioned by issue #20):
    k_nh4_small
    k_nh4_diazo
    k_nh4_large
    k_no3_small
    k_no3_diazo

    I'm still keeping the do_14c option and all the diagnostics related to radiocarbon. We can remove them later if we decide to go in that direction.

This PR should not change baselines (passed CI test) and can produce b2b diagnostic netCDF files.

Eventually we could apply this refactoring to the majority of existing generic_COBALT subroutines (e.g., PR #22 ), which will enhance the clarity and conciseness of the main COBALT code.

yichengt900 commented 2 months ago

This PR addresses all the comments from reviewers and is up-to-date with the main branch. Please review it again, and feel free to provide any further comments, thanks.

charliestock commented 2 months ago

Thanks for all of your hard work on the Yi-Cheng, all. It looks good from my end.

yichengt900 commented 2 months ago

Thanks, @charliestock. I've also merged #39 into this PR. It's ready for merging once one of you approves it.