marbl-ecosys / MARBL

Marine Biogeochemistry Library
https://marbl-ecosys.github.io
Other
14 stars 25 forks source link

Adding SPECTRA functionality into MARBL development code #396

Closed jessluo closed 2 years ago

jessluo commented 2 years ago

Split zoo_loss into linear and aggregate components; f_zoo_detr only acts on aggregate losses.

mnlevy1981 commented 2 years ago

This PR is failing the continuous integration because one of the lines added is too long -- can you break it into two lines?

$ git diff
diff --git a/src/marbl_interior_tendency_mod.F90 b/src/marbl_interior_tendency_mod.F90
index 4e0d8cc..f351091 100644
--- a/src/marbl_interior_tendency_mod.F90
+++ b/src/marbl_interior_tendency_mod.F90
@@ -2018,7 +2018,8 @@ contains
         do zoo_ind = 1, zooplankton_cnt
           zoo_loss_poc(zoo_ind,k) = f_zoo_detr(zoo_ind,k) * zoo_agg_loss(zoo_ind,k)
           zoo_loss_doc(zoo_ind,k) = (c1 - parm_labile_ratio) * (c1 - f_zoo_detr(zoo_ind,k)) * zoo_agg_loss(zoo_ind,k)
-          zoo_loss_dic(zoo_ind,k) = (parm_labile_ratio * (c1 - f_zoo_detr(zoo_ind,k)) * zoo_agg_loss(zoo_ind,k)) + zoo_linear_loss(zoo_ind,k)
+          zoo_loss_dic(zoo_ind,k) = (parm_labile_ratio * (c1 - f_zoo_detr(zoo_ind,k)) * zoo_agg_loss(zoo_ind,k)) &
+                                  + zoo_linear_loss(zoo_ind,k)
         end do

         !-----------------------------------------------------------------------

(If you'd prefer, I can try to make the change myself -- I think I have push permissions to the branch now)

jessluo commented 2 years ago

Currently generating a conservation error with carbon. Possibly due to the subtraction of zoo_loss_basal from zoo_loss in the routing computation. Test out a scenario where zoo_loss is zoo_loss_bulk and the 3D zoo_loss diagnostic adds together zoo_loss_bulk and zoo_loss_basal.

mnlevy1981 commented 2 years ago

I've verified that c2c11da is failing the CI tests because we added the zoo_loss_basal, zoo_loss_basal_zint, and zoo_loss_basal_zint_100m diagnostics -- existing diagnostics are unchanged by this PR. To fix the failing test, can you please copy /glade/u/home/mlevy/for_jluo/call_compute_subroutines.history.nc to ${MARBL}/tests/input_files/baselines/?

Full disclosure: this file also includes a round-off level change to OtherRemin that I suspect are due to compiler changes on my laptop, but might be traced to a PR that we've merged since the last baseline was made.

mnlevy1981 commented 2 years ago

So I think the remaining tasks for this PR are:

  1. I will run this through the CESM aux_pop_MARBL test suite
  2. @jessluo will update spectra-config to account for the new PFT settings that have been introduced since this PR began, and I'll include the updated user_nl_marbl as tests/input_files/settings/marbl_spectra.settings

I might also update the CI test suite to include running init.py with the updated spectra settings. If a future PR changes PFT settings further, that test will fail and we'll know we need to update spectra-config accordingly.

@jessluo -- did I miss anything from our conversation the other day? I think you have some items on your plate for spectra-config that won't affect this PR

mnlevy1981 commented 2 years ago

The CESM test suite finished, and it did highlight one more issue: we need to add

        basal_metabolic_rate_per_day :
           longname : Basal metabolic rate
           subcategory : 11. zooplankton
           units : 1/day / (mmol/m^3)
           datatype : real
           default_value :
              default : 1e34
              ((zooplankton_sname)) == "zoo" : 0.0

to settings_cesm2.0.yaml, settings_cesm2.1+cocco.yaml, settings_cesm2.1.yaml, and settings_latest+cocco.yaml. The tests using OCN_BGC_CONFIG=latest+cocco failed with

19:(Task 1, block 1) MARBL ERROR (marbl_settings_mod:add_var): User must provide value for zooplankton_settings(1)%basal_metabolic_rate_per_day via put_setting()
19:(Task 1, block 1) MARBL ERROR (marbl_settings_mod:marbl_settings_define_PFT_derived_types): Error reported from this%add_var(zooplankton_settings(1)%basal_metabolic_rate_per_day)
19:(Task 1, block 1) MARBL ERROR (marbl_init_mod:marbl_init_parameters_pre_tracers): Error reported from marbl_settings_define_PFT_derived_types()
19:(Task 1, block 1) MARBL ERROR (marbl_interface:init): Error reported from marbl_init_parameters_pre_tracers
19:(Task 1, block 1) MARBL ERROR (ecosys_driver:ecosys_driver_init): Error reported from marbl(1)%init()

I also want to update the stand-alone test suite to highlight issues like this, so I'll try to make a couple of commits to this PR. @jessluo I might reach out to ask for write permissions to this branch.

mnlevy1981 commented 2 years ago

I've rerun aux_pop_MARBL with 1915830 and all the failures are expected:

  1. The three new per-zoo diagnostics in POP's history file cause all of the tests to fail baseline comparisons with
SUMMARY of cprnc:
 A total number of   1231 fields were compared
          of which      0 had non-zero differences
               and      0 had differences in fill patterns
               and      0 had different dimension sizes
 A total number of      3 fields could not be analyzed
 A total number of      3 time-varying fields on file 1 were not found on file 2.
 A total number of      0 time-constant fields on file 1 were not found on file 2.
 A total number of      0 time-varying fields on file 2 were not found on file 1.
 A total number of      0 time-constant fields on file 2 were not found on file 1.
  diff_test: the two files DIFFER only in their field lists
  1. I modified user_nl_MARBL in POP's ecosys_spectra_pfts test to match marbl_with_spectra.settings in this PR, so SMS_Ld2_D.T62_g17.C1850ECO.cheyenne_gnu.pop-ecosys_spectra_pfts and SMS_Ld2_D.T62_g17.C1850ECO.cheyenne_intel.pop-ecosys_spectra_pfts fail baseline comparisons due to changing answers in addition to introducing the new diagnostics
SUMMARY of cprnc:
 A total number of   2021 fields were compared
          of which   1682 had non-zero differences
               and      0 had differences in fill patterns
               and      0 had different dimension sizes
 A total number of      3 fields could not be analyzed
 A total number of     18 time-varying fields on file 1 were not found on file 2.
 A total number of      0 time-constant fields on file 1 were not found on file 2.
 A total number of      0 time-varying fields on file 2 were not found on file 1.
 A total number of      0 time-constant fields on file 2 were not found on file 1.
  diff_test: the two files seem to be DIFFERENT
  1. The new MARBL setting is causing all the tests to fail namelist comparison as well
Inequivalent lines zooplankton_settings(1)%lname = 'Zooplankton' != zooplankton_settings(1)%basal_metabolic_rate_per_day = 0.0
  NORMALIZED: zooplankton_settings(1)%lname = 'Zooplankton' != zooplankton_settings(1)%basal_metabolic_rate_per_day = 0.0
jessluo commented 2 years ago

corresponding changes are also made on spectra-config: https://github.com/jessluo/spectra-config/commit/5c333eb78182d9f392889e01fb4239ee0e3f745d