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

Split marbl_mod and marbl_ciso_mod #308

Merged

Conversation

mnlevy1981
Copy link
Collaborator

Separate the initialization, surface flux computation, and interior tendency computations in both marbl_mod.F90 and marbl_ciso_mod.F90: the end result is replacing those two modules with seven new ones and moving some routines to marbl_init_mod.F90:

  1. marbl_surface_flux_mod -- contains marbl_surface_flux_compute(), formerly marbl_set_surface_forcing()
  2. marbl_interior_tendency_mod -- contains marbl_interior_tendency_compute(), formerly marbl_set_interior_forcing(). Also has marbl_set_global_scalars_interior(), which needs to be renamed (ideally it would be moved to marbl_glo_avg_mod but it calls adjust_bury_coeff() which needs to stay in marbl_interior_mod)
  3. marbl_glo_avg_mod -- contains marbl_glo_avg_count_vars() and marbl_glo_avg_init_rmean_vals()
  4. marbl_ciso_init_mod -- contains marbl_ciso_init_tracer_metadata()
  5. marbl_ciso_surface_flux_mod -- contains marbl_ciso_surface_flux_compute(), formerly marbl_ciso_set_surface_forcing()
  6. marbl_ciso_interior_tendency_mod -- contains marbl_ciso_interior_tendency_compute(), formerly marbl_ciso_set_interior_forcing().
  7. marbl_interior_share_mod -- contains marbl_interior_share_update_sinking_particle_from_level_above(), which used to be a private routine (marbl_update_sinking_particle_from_prior_level()) in both marbl_mod and marbl_ciso_mod; now both tracer modules can use the same code. Perhaps this would also be a good place for adjust_bury_coeff(), allowing marbl_set_global_scalars_interior() to be moved to marbl_glo_avg_mod?

For consistency, the interface routines set_surface_forcing() and set_interior_forcing() have been renamed surface_flux_compute() and interior_tendency_compute(), respectively, and renames marbl_comp_nhx_surface_emis() to marbl_nhx_surface_emis_compute().

Addresses #86

Next will be to create marbl_interior_mod and pull out the interior routines.

Note that this commit does not build - other modules are still trying to use
marbl_mod, which no longer exists.
marbl_glo_avg_mod: has marbl_glo_avg_count_vars and
                   marbl_glo_avg_init_rmean_vals (both called during init()) as
                   well as module variables used by next two modules
marbl_surface_mod: has set_surface_forcing
marbl_interior_mod: has set_interior_forcing and
                    marbl_set_global_scalars_interior as well as lots of small
                    private functions called by those two

Note: need to come up with better names for functions in marbl_surface_mod and
marbl_interface_mod. One idea is to rename the modules again:

marbl_surface_mod -> marbl_surface_fluxes_mod [with
                     marbl_surface_fluxes_compute]
marbl_interior_mod -> marbl_interior_tendencies_mod [with
                      marbl_interior_tendencies_compute]; unsure of
                      marbl_set_global_scalars_interior, though. Maybe
                      marbl_interior_set_global_scalars? (maybe rename
                      functions but not the modules?)
Also add a use statement to set_interior_forcing(); missing
marbl_diagnostics use statement leads to linking error though libmarbl.a
builds fine.
interface: set_surface_forcing() -> compute_fluxes()
surface: marbl_set_surface_forcing() -> marbl_surface_compute_fluxes()
ciso: marbl_ciso_set_surface_forcing() -> marbl_ciso_compute_fluxes()

interface: set_interior_forcing() -> compute_fluxes()
interior: marbl_set_interior_forcing() -> marbl_interior_compute_tendencies()
ciso: marbl_ciso_set_interior_forcing() -> marbl_ciso_compute_tendencies()
Function is now marbl_nhx_surface_emis_compute(), conforming to the convention
that all public functions in marbl_XYZ_mod have a name marbl_XYZ_[something]()
It is now marbl_surface_flux_mod, and marbl_surface_compute_fluxes() is now
marbl_surface_flux_compute(). Similarly, marbl_interface%compute_fluxes() is
now marbl_interface%surface_flux_compute().
It is now marbl_interior_tendency_mod, and marbl_interior_compute_tendencies()
is now marbl_interior_tendency_compute(). Similarly,
marbl_interface%compute_tendencies() is now
marbl_interface%interior_tendency_compute().
Moved to marbl_ciso_surface_flux_mod, and next commit will split it to also
create marbl_ciso_init_mod and marbl_ciso_interior_tendency_mod
Now resides in marbl_ciso_init_mod; also created
marbl_ciso_interior_tendency_mod although it is empty as of this commit.
This completes the split of marbl_ciso_mod. I noticed that
marbl_interior_tendency_mod and marbl_ciso_interior_tendency_mod both had
similar update_particulate_terms_from_prior_level() routines that called
identical marbl_update_sinking_particle_from_prior_level() routines, so I moved
the latter to a new module (marbl_interior_share_mod) amd renamed it
marbl_interior_share_update_sinking_particle_from_level_above()
flux_as and flux_sa were both set but then not used in
marbl_ciso_compute_fluxes()
@mnlevy1981
Copy link
Collaborator Author

Still to do: clean up the module-level comments in all these new modules. The original comments from marbl_mod.F90 ended up in marbl_surface_flux_mod.F90, and the comments from marbl_ciso_mod.F90 are in marbl_ciso_surface_flux_mod.F90 . It's not entirely clear to me where documentation like the following belongs:

  !  Multispecies ecosystem based on Doney et al. 1996, Moore et al., 2002
  !  Based on POP Global NCAR Nitrogen Ecosystem Model
  !  version 0.0 (June 15th, 1998) from S.C. Doney.
  !  Based on Doney et al., 1996 model.
  !  Climate and Global Dynamics, NCAR
  !  ([email protected])
  !
  !  Version 1.0
  !  Multispecies, multiple limiting nutrient version of ecosystem
  !  based on mixed layer model of Moore et al.(2002).  Implemented here with
  !  fixed elemental ratios and including only the diatoms and small
  !  phytoplankton, with a parameterization of calcification,
  !  by Keith Lindsay and Keith Moore, Fall 2001 - Spring 2002.
  !  Calcification parameterization based on Moore et al. 2002.
  !
  !  Version 2.0, January 2003
  !    Adds diazotrophs as a phytoplankton group, (based on Moore et al., 2002a)
  !    Allows for variable fe/C for all phytoplankton groups
  !     Allows for variable si/C for the diatoms
  !     Adds explicit tracers for DON, DOP, DOFe
  !     variable remin length scale for detrital soft POM and bSi f(temperature)
  !     Extensive modifications to iron scavenging parameterization
  !     Addition of a sedimentary dissolved iron source,
  !        (implemented in ballast code as excess remin in bottom cell)
  !        coded by J.K. Moore, ([email protected])
  !
  !   Version 2.01. March 2003
  !     corrected O2 bug
  !     corrected grazing parameter z_grz bug at depth
  !     dust dissolution at depth releases iron,
  !     increased length scale for dust diss., increased hard fraction dust
  !     no deep ocean reduction in scavenging rates,
  !     increase bSi OC/ballast ratio 0.3 -> 0.35,
  !     corrected bug in diazotroph photoadaptation, and diat and sp adapatation
  !
  !   Version 2.02.
  !     corrected bug in Fe_scavenge (units for dust), May 2003
  !     changed C/N/P ratios to 117/16/1 (Anderson & Sarmiento, 1994)
  !
  !   Version 2.03., July 2003
  !     Remin of DOM no longer temperature dependent,
  !     new iron scavenging parameterization added,
  !     some dissolution of hard fraction of ballast materials added
  !
  !   Version 2.1, September 2003
  !     modfied iron scavenging and dust dissolution at depth
  !
  !   Version 2.11, March 2004
  !     fixed bug in iron scavenging code, replace dust and POC flux_in w/ flux_out
  !
  !   Version 2.12, April 2004 - Final version for GBC paper revision,
  !     (Questions/comments, Keith Moore - [email protected]
  !
  !   References
  !   Doney, S.C., Glover, D.M., Najjar, R.G., 1996. A new coupled, one-dimensional
  !   biological-physical model for the upper ocean: applications to the JGOFS
  !   Bermuda Time-Series Study (BATS) site. Deep-Sea Res. II, 43: 591-624.
  !
  !   Moore, JK, Doney, SC, Kleypas, JA, Glover, DM, Fung, IY, 2002. An intermediate
  !   complexity marine ecosystem model for the global domain. Deep-Sea Res. II, 49:
  !   403-462.
  !
  !   Moore, JK, Doney, SC, Glover, DM, Fung, IY, 2002. Iron cycling and nutrient
  !   limitation patterns in surface waters of the world ocean. Deep-Sea Res. II,
  !   49: 463-507.

Also, there are two things I want to look into doing:

  1. Split marbl_diagnostics_mod.F90 into marbl_diagnostics_mod.F90 and marbl_ciso_diagnostics_mod.F90
  2. Always call marbl_ciso_*() routines, but have if (.not. ciso_on) return at the top... i.e. move the if (ciso_on) checks into the Ciso routines. I think that will be a cleaner template to follow when adding additional tracer modules such as Niso.

marbl_ciso_surface_flux_compute() and marbl_ciso_interior_tendency_compute()
both return immediately if ciso_on is false, so marbl_surface_flux_compute()
and marbl_interior_tendency_compute() can both call those subroutines without
checking the value of ciso_on first.

Also, I moved two routines that pack share data types to
marbl_interior_share_mod.F90 -- they get called regardless of ciso_on but
return immediately if ciso_on is false.

There are still a few places where we check the value of ciso_on that I would
prefer to avoid, but that will require a little more refactoring to make
possible.
Created marbl_ciso_diagnostics_mod.F90 to handle the ciso diagnostics as well
as marbl_diagnostics_share_mod.F90 to house subroutines / datatypes that are
used by both ciso and the base tracers.
nag caught three unnecessary use statements in
marbl_ciso_diagnostics_mod.F90
@mnlevy1981
Copy link
Collaborator Author

I've split marbl_diagnostic_mod.F90, and now realize that there are lots of variables, data types, and subroutines still using surface_forcing and interior_forcing -- I will be renaming all of those to surface_flux and interior_tendency, respectively.

Always call marbl_ciso_init_tracer_metadata, return immediately if ciso_on is
.false.
@mnlevy1981
Copy link
Collaborator Author

To clarify the last comment, I want to replace surface_forcing with surface_flux in relationship to the computation done in marbl_surface_flux_compute() (formerly set_surface_forcing()). Datatypes and subroutines dealing with forcing fields requested by MARBL (u10_sqr, sst, Salinity, etc) should keep forcing in its moniker, and we do separate those forcings based on whether they apply to the surface flux computation or the interior tendency computation. Maybe those should be referred to forcings_for_surface_flux and forcings_for_interior_tendency? That's starting to get pretty long, especially for things like marbl_forcings_for_surface_flux_indexing_type and marbl_forcings_for_interior_tendency_indexing_type

This may not be the final name change, I think we need to figure out a better
name than "surface flux output" (it's confusing because surface_tracer_flux is
the primary output of surface_flux_compute() and also Chl isn't a flux but is
stored in this structure)
num_elements_surface_flux is a better name for the number of columns being
computed simultaneously inside surface_flux_compute().
We don't use _tracer in the marbl_surface_flux_mod name or any subroutines, so
I'm removing it from the variable as well. I also removed the "stf" shorthand,
just using surface_fluxes everywhere.
Also added a comment that this datatype is designed to pass data from
surface_flux_compute() to marbl_diagnostics_mod.F90
1. marbl_diagnostics_set_surface_forcing() is now
   marbl_diagnostics_surface_flux_compute()
2. store_diagnostics_ciso_surface_forcing() is now
   marbl_ciso_diagnostics_surface_flux_compute()
! Compute carbon isotopes surface fluxes
!-----------------------------------------------------------------------

! pass in sections of surface_input_forcings instead of associated vars because of problems with intel/15.0.3
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having the if (ciso_on) check in marbl_ciso_surface_flux_mod means I'm always trying to pass surface_input_forcings(ind%d1{3,4}c_id), and those indices are 0 for non-ciso runs. So I think we should pass all of surface_input_forcings (as well as ind) and let the subroutine pull out sst, d13c, and d14c.

Since the check to see if ciso tracers are enabled is now in this routine, we
can not specify surface_input_forcings(surface_forcing_ind%d13c_id)%field_0d in
the argument list (because non-ciso runs will try try to pass
surface_input_forcings(0)%field_0d, which is out of range).
Lots of comments and a few log messages still used "surface forcing" rather
than "surface flux". As of this commit, I believe that all references to
surface_forcing refer to the forcing fields requested by MARBL for the
surface_flux_compute() routine.

Next commit will be to rename surface_input_forcings to just plain
surface_forcings as that is no longer an [incorrectly] overloaded term.
Now marbl_interior_tendency_share_type (matches marbl_surface_flux_share_type).
Also, local variables of this type are now marbl_interior_tendency_share
It is now interior_tendency_forcing_ind (same with indexing_type).
dtracers, column_dtracers, etc are all now interior_tendencies. The subroutine
compute_dtracer_local() is now compute_local_tendencies().
Variable is now interior_tendency_forcings
Now num_interior_tendency_forcing_fields
All occurences are now interior_tendency_forcing
Now tracers and tracers_at_surface, respectively.
All modules have a module-level "implicit none" so individual subroutines in
the modules do not need it.
Expect 'interior_tendency' instead of 'interior' and 'surface_flux' instead of
'surface'
Renamed to autotroph_zero_consistency_enforce and also moved the ciso check to
marbl_ciso_interior_tendency:marbl_ciso_autotroph_set_to_zero
* Renamed marbl_interior_share_mod to marbl_interior_tendency_share_mod
  (also updated public subroutines in the module)
* Changed ciso_on check in share modules from "if (.not. ciso_on)
  return" to "if (ciso_on) then" blocks [more extensible once we add
  other tracer modules]
* Created marbl_surface_flux_share_mod.F90 with an export_variables
  function to remove ciso_on from marbl_surface_flux_mod.
This removes ciso_on from marbl_interior_tendency_mod
marbl_interior_tendency_share_update_sinking_particle_from_level_above() is too
long for a function name (NAG warns about more than 63 characters and gfortran
throws an error (possibly because we elevate warnings to errors).

New name is marbl_interior_tendency_share_update_particle_flux_from_above()
num_elements_surface -> num_elements_surface_flux
num_elements_interior -> num_elements_interior_tendency
num_interior_tendency_forcings -> num_elements_interior_tendency
set_global_scalars (from marbl_interface_class) can call adjust_bury_coeff
directly. Now named marbl_interior_tendency_adjust_bury_coeff, the subroutine
returns immediately if ladjust_bury_coeff is .false.
Now marbl_glo_avg_var_cnts_compute()
Variables and datatypes both relied on surface_saved_state and
interior_saved_state rather than more descriptive names.
Now marbl_interior_tendency_diag_ind; also, included _tendency in type name
Instead of marbl_diagnostics_share_mod, these belong in
marbl_interface_private_types.
This should be the last of the code clean-up commits following review of marbl-ecosys#308
f2ff0b1 updated marbl_glo_avg_mod.F90 but I didn't change the names of
interface variables.
This commit renames the glo_scalar variables on the interface.
Lots of work has been done to the interface, and that is now reflected in the
documentation.
@mnlevy1981 mnlevy1981 merged commit 2dd964c into marbl-ecosys:development Aug 29, 2018
@mnlevy1981 mnlevy1981 mentioned this pull request Aug 29, 2018
@mnlevy1981 mnlevy1981 deleted the enhancement/split_marbl_mod branch August 29, 2018 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant