NOAA-CEFI-Regional-Ocean-Modeling / ocean_BGC

3 stars 8 forks source link

Clean up and simplify bacterial production #66

Closed charliestock closed 5 months ago

charliestock commented 5 months ago

This pull request addresses the lion's share of the cleanup and simplification of bacterial production raised in issue 63. Most critically, it removes code associated with a preliminary exploration of the contributions of nitrification and anammox to bacterial production.

Parameters removed included the growth efficiencies for anammox and nitrification (amx_ge and nitrif_ge). Other changes included:

  1. Removing the uptake of inorganic phosphate associates with chemoautotrophic production (bact%juptake_po4)
  2. Removing three types of bacterial production (bact%jprod_n_het, bact%jprod_n_amx and bact%jprod_n_nitrif) in favor of one (bact%jprod_n).
  3. Similarly reverting the bact%mu_h to bact%mu
  4. Removing mu_cstar, which was used to estimate the fraction of cheoautotrophs
  5. Renamed jprod_n2amx to jnamx to remove ambiguity in units
  6. Revert to simple nitrification and anammox stoichiometries and added improved parameter descriptions and units for these variables
  7. Simplifying and improved commenting bacterial production calculations in generic_COBALT.F90

This should cleanup should not change answers. One additional change, however, is required to fully remove the autotrophy and this will change the details of how residual bacterial populations in poor environments (negative growth) decay in the model. I have saved this for a second pull request, which will change answers.

The code ran successfully in a 24 hour test run. It did require removing wc_vert_int_chemoautopp from the default diag_table and replacing cobalt%wc_vert_int_jprod_n2amx with wc_vert_int_jnamx. It also required COBALT_input and COBALT_override options to be added to the input and blank files created following the revised parameter inputs.

charliestock commented 5 months ago

Have we been able to test to see if there are answer changes on this? If so, perhaps it would make sense for me to add a commit with the relatively small final change so that Jessica and Gaby can do their review?

yichengt900 commented 5 months ago

@charliestock, the current commits did not change the answers in both the automated 1D test and the NWA12 RT (manual test). I will need to update our experiments in the CEFI-regional-MOM6 repository to include COBALT_input/COBALT_override as well as the updated diag_table so the automated testing can work correctly again. Please feel free to add any new commits you want.

jessluo commented 5 months ago

So my understanding is that this change essentially reverts the bacteria code back to COBALTv2? In that case, would it be possible to do a line by line comparison with https://github.com/NOAA-GFDL/ocean_BGC/blob/011b72482e4c7c09c46e26719b79c5324eb02d69/generic_tracers/generic_COBALT.F90#L6983-L7021 here in github?

jessluo commented 5 months ago

Comparing the code side by side, it looks like the main difference is the replacement of bact(1)%f_n in the juptake_ldon and jprod_n calculations in COBALTv2 with bact(1)%bhet, which looks to be zero under conditions where growth is negative. @charliestock, is this the part of the code that changes answers that you also wanted to revert?

charliestock commented 5 months ago

Hi Jessica,

Yes the last step will be to get rid of bhet, but that will change answers, at least by a bit. I will add this as an additional commit today, and then I think a comparison with the COBALTv2 code should be pretty much the same, plus or minus the improved comments, etc.

Cheers, Charlie

jessluo commented 5 months ago

This looks great. The code looks like what we had before in COBALTv2, reverting the changes adding chemoautotrophic bacteria production.

yichengt900 commented 5 months ago

Thanks @charliestock , @jessluo and @gabyneg. I will need to merge this CEFI-regional-MOM6's PR to make the automated testing works again. I also confirm that although this PR does not change the answers in our 1D test, it does change the answers in our NWA12-RT case. I will update the reference in a separate PR.

charliestock commented 5 months ago

Hi folks,

I have added a third commit that makes the final reversions back to the original code. I have confirmed that the code is the same as the COBALTv2 code with the exception of comments and some cleaning up of the spacing. I did this by putting the old and new sections into files at:

/autofs/ncrc-svm1_home1/Charles.Stock/COBALTv3_dev/bacteria_cleanup/diff_test

doing module load tkdiff, and then "tkdiff Cobaltv3_Bacteria Old_Cobalt_Bacteria".

The answer changes will arise from the fact that we have taken out all code associated with the diagnosis of the heterotrophic bacterial biomass relative to the total bacterial biomass. This involved comparing the heterotrophic bacterial growth rate to the inferred chemoautotrophic "turnover". The latter quantity was set to 0 in our runs, so answers will identical for all growth rates > 0. It is possible, however for the bacterial growth rate to be < 0, at least for a short time, under unfavorable conditions. Some differences may arise under these conditions, but these are by definition areas where bacteria are having minimal impact because they are unable to sustain themselves.

Anyway, if @jessluo and @gabyneg could have one final peek a the bacterial production section after this third commit, that would be great. It would good to be back to the original simple code in this section!

thank you, Charlie

jessluo commented 5 months ago

Looks good to me!

yichengt900 commented 5 months ago

Thank you, @charliestock, for the detailed description, and @jessluo for confirming that all the changes are good. I fixed the automated testing and now this PR passed it. I will merge this PR now. Thanks again everyone.

charliestock commented 5 months ago

Thank you @yichengt900, @gabyneg and @jessluo