Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix/240 #263

Merged
merged 14 commits into from
Apr 21, 2018
Merged

Bugfix/240 #263

merged 14 commits into from
Apr 21, 2018

Conversation

mnlevy1981
Copy link
Collaborator

Added additional terms to zooplankton_share_type so that marbl_ciso_mod.F90 could include zoo_graze terms when computing tracer tendencies. This fixes #240 in the "quick fix" sense, but @klindsay28 and I talked about how it makes sense for me to take a little extra time to clean up the way some of this data is passed from marbl_mod.F90 to marbl_ciso_mod.F90. Namely:

  1. Most (all?) of autotroph_share_type is a direct copy of autotroph_secondary_species_type; why don't we pass that instead of doing unnecessary memory copies?
  2. marbl_ciso_mod.F90 is only concerned with the sum over all zooplankton species, so this summation should be done as part of building zooplankton_share_type rather than done internal to marbl_ciso_mod.F90. That would be consistent with how DOCtot is handled (summed in marbl_mod.F90, treated as a single quantity throughout marbl_ciso_mod.F90)

I'll add another comment when this is actually ready for review.

Several tendencies in marbl_ciso_mod.F90 are missing zoo_graze terms; the first
step to add them in is to make them accessible in the module via the share
type.
Previously, marbl_ciso_mod.F90 computed tendencies without considering
the possibility that zooplankton could be grazed, so no zoo_graze_*
terms were included. The mass balance check did not catch this (even in
a test run where zoo grazes itself) because the way the mass balance
checks are constructed the missing terms all canceled each other out.

I did confirm that adding some zoo_graze terms (but not all of them)
triggered the error when zoo grazed itself and had no effect when zoo
was not grazed.
marbl_ciso_mod.F90 only wants the sum of zooplankton terms like zoo_loss
and zoo_graze (sum over all zooplankton species). Instead of sending
each individual loss / grazing term, marbl_mod.F90 now does the sum in
marbl_export_zooplankton_shared_variables().

Also, marbl_ciso_mod.F90 does not need x_graze_zoo so that field is not
included in the share type.
Instead of copying these into marbl_autotroph_share_type, just pass
autotroph_secondary_species to marbl_ciso_set_interior_forcing().
Cleaned up more variables that were available to marbl_ciso_mod.F90 via
autotroph_secondary_species but were still being copied to
marbl_autotroph_share_type. At this point, the only use of the share
type is to copy tracer_local terms and there is already a plan to pass
tracer_local instead.
@mnlevy1981
Copy link
Collaborator Author

marbl_zooplankton_share_type is now used to pass the sum over all zooplankton of fields marbl_ciso_mod.F90 needs. marbl_autotroph_share_type only passes tracer values, and autotroph_secondary_species is passed directly instead of being copied into other components of the share type. Note that the share type can be removed after we resolve #243.

Part of some post-review code-cleanup - realized that the
zooplankton_share_type and marbl_ciso_mod.F90 should be using zootot instead of
zoo to signify this is a sum over zooplankton species. As a first pass, I
changed zoo13Ctot and zoo14Ctot to zootot13C and zootot14C, respectively.
Since marbl_mod.F90 sums over all zooplankton when storing data in the
zooplankton share type, those variables are now refered to as zootot:

zoototC
zootot_loss
zootot_graze

And so on. This is most pronounced in marbl_ciso_mod.F90, but also is apparent
in the diagnostics module.
The changes to diagnostic names and metadata from the previous commit
were not included in default_diagnostics.yaml, so GCMs were looking for
CISO_zooC_d1[34]C instead of CISO_zoototC_d1[34]C
Expand autotroph_local_type to include carbon isotope elements, and then pass
autotroph_local to marbl_ciso_set_interior() instead of copying data into the
share type. Other related clean-ups:

* marbl_autotroph_share_type is no longer needed
* marbl_compute_autotroph_phyto_diatoms() does not need autotroph_local
* marbl_ciso_mod.F90 no longer needs its own autotroph_local_type, so there is
  no longer a consistency check there (or a setup_autotroph_local, since that
  is taken care of in marbl_mod)
tracer_local is no longer restricted to the base tracers, it also
contains carbon isotope tracers if ciso_on = .true. This fixes a bug in
attempts to put carbon isotope fields in autotroph_local.
No need to compute this ratio for autotrophs that do not have CaCO3
tracer
Instead of copying some elements of tracer_local to interior_share and
recomputing other elements in setup_local_column_tracers, just pass
tracer_local (instead of tracers) to marbl_ciso_set_interior_forcing(). This
means that some associate statements need to be updated to point to elements of
tracer_local instead of marbl_interior_share_type (and some local variables are
now associate statements instead of being computed in marbl_ciso_mod).
DO13Ctot_loc was inadvertently pointing to DO14Ctot_loc
@mnlevy1981
Copy link
Collaborator Author

a284a46 addresses #243 (d700590 has most of the infrastructure, but a typo where DO13Ctot_loc was pointing to DO14Ctot_loc)

* several "zooC" -> "total zooC" or "zoototC" in comments
* R1[34]C_zooC -> R1[34]C_zoototC
* autotroph consistency check has a single "if (ciso_on) then" block
  and checks for Ca1[34]CO3_ind instead of individual checks for all
  isotopic autotroph tracers
* ciso_set_interior_forcing(): reordered arguments in interface so all
  intent(in)s are clumped together, followed by the (inout)s
@mnlevy1981 mnlevy1981 merged commit d106c6d into marbl-ecosys:master Apr 21, 2018
@mnlevy1981 mnlevy1981 deleted the bugfix/240 branch April 23, 2018 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant