marbl-ecosys / marbl Goto Github PK
View Code? Open in Web Editor NEWMarine Biogeochemistry Library
Home Page: https://marbl-ecosys.github.io
License: Other
Marine Biogeochemistry Library
Home Page: https://marbl-ecosys.github.io
License: Other
Right now, marbl_saved_state_type
looks like
type, public :: marbl_saved_state_type
real (r8), allocatable :: ph_prev_col(:) ! (km)
real (r8), allocatable :: ph_prev_alt_co2_col(:) ! (km)
real (r8), allocatable :: ph_prev_surf(:) ! (num_elements)
real (r8), allocatable :: ph_prev_alt_co2_surf(:) ! (num_elements)
contains
procedure, public :: construct => marbl_saved_state_constructor
end type marbl_saved_state_type
We need to generalize this in a way similar to what we have done with the diagnostics - remove the hard-coded field names in favor of something where we can easily add new variables that need to be saved from time step to time step when needed. This lets the driver allocate memory at runtime for the saved state variables, which has two main benefits:
Currently, ecosys_driver_init_tracers_and_saved_state()
applies the land mask to ecosys tracers, whie ecosys_driver_ciso_init_tracers()
applies land mask to ciso tracers. Why not apply land mask all at once after initializing all tracers?
Are there any forcing fields that are required by MARBL? (Or perhaps required in specific configurations?) If so, we should check to make sure their corresponding indices are non-zero at the end of marbl_mod::marbl_init_surface_forcing_fields
(after all the field definitions).
have MARBL specify, in a generic sense, which fields it needs,
but specification of forcing field source resides in driver
FIXME comment in marbl_co2calc_mod::drtsafe
:
make marbl_status_log required - this is currently needed
since abio_dic_dic14_mod is calling this routine but has
not itself been MARBLized yet
FIXME comment in marbl_co2calc_mod::marbl_comp_co3_coeffs
:
the computations for the individual constants need to
be broken out into separate functions and unit tested
We want to pass error codes out of MARBL so that the driver can handle them. There is already a datatype in marbl_interface_types.F90
:
type, public :: marbl_status_type
integer :: status
character(marbl_str_length) :: message
end type marbl_status_type
And it is used in a few places (e.g. marbl_ecosys_init_nml
) but it needs to be implemented more widely.
We are, in different modules, referring to the surface forcing is surface
, sflux
, and forcing
. Rather than have three different terms for the same thing, we need one term used everywhere. Keith's vote is to change them all to surface_forcing
, because it is accurate and descriptive.
Two examples of 3D fields that are read from files and used in computing the interior tendencies are iron sediment flux and tracer restoring (but there probably are other uses and / or other uses coming in future developments). We (by which I mean "@klindsay28 and I") talked about a general set up where MARBL can request that the driver reads in these 3D fields without the driver knowing what the fields will be used for. Using phosphate restoring as an example, the workflow would look something like this:
marbl_init
that PO4 should be restored to a climatology read from PO4_clim_var
in PO4_clim_file
, so MARBL will populate an element in the interior_forcing array containing the necessary metadataecosys_driver_init
will loop through the entire interior_forcing array (including our specific element containing the PO4 restoring information) and read the files into a global array.ecosys_driver_set_interior
will send the correct column of the global data to marbl_set_interior
marbl_set_interior
will recognize that the index in the interior forcing array corresponding to phosphate restoring is non-zero, so it will call compute_interior_restore_tendency
with the PO4 tracer value and the climatology passed through from the driverSo marbl_init
would have a section of code that looks like
if (PO4_restore) then
call interior_forcing_fields%add_forcing_field(field_source='PO4_clim_file', &
field_varname='PO4_clim_var', id=tracer_restore_ind(PO4_ind))
end if
and marbl_set_interior
would have
do nt=1,ecosys_used_tracer_cnt
if (tracer_restore_ind(nt).ne.0) then
call marbl_restore(...)
end if
end do
Again, all the PO4_*
variables are just placeholders - I think we actually have arrays of these variables (of length ecosys_used_tracer_cnt
) coming in via the ecosys_restore_nml
namelist.
A template for pull requests would let us provide developers with a check-list to make sure the code has been properly tested. Things to include in check list:
Now that we are doing things in a column-by-column manner rather than level-by-level, we can move the contents of marbl_ecosys_set_sflux
and marbl_ecosys_set_interior
to marbl_set_surface_flux
and marbl_set_interior
, respectively. (Also, see Issue #5 regarding marbl_set_surface_flux
.)
Right now MARBL has adopted the cgs
unit convention from POP, but we want to enforce mks
. This will require conversion of intent(in) and intent(out) in the driver, and will also requirement conversion in the diagnostics.
call marbl_get_param_list()
(or marbl_[module]_get_param_list()
); some sort of access method to tell the GCM what parameters exist / what their values are. Potentially add ability to overwrite?
Currently passing marbl_mod::marbl_compute_PAR
entire autotroph_local_type
array when we just need the Chl
attribute of it.
In the driver, POP currently declares
type(marbl_interface_class) :: marbl
This makes searching for anything related to the instance difficult. Perhaps
type(marbl_interface_class) :: marbl_instance
is more descriptive? And, while we're at it, maybe marbl_instance_class
instead of marbl_interface_class
?
Most autotroph diagnostics have a shortname of
sname = trim(autotrophs(n)%sname) // '_ABCDEF'
for some value of ABCDEF
, but Si Uptake omits the _
and sets
sname = trim(autotrophs(n)%sname) // 'bSi_form'
This is the first operation we've come across that violates the "single column" aspect of MARBL - cases where MARBL may need the driver to compute global integrals and return the values. @klindsay28 and I discussed it this afternoon, and think the following are all reasonable:
gcm_global_sum
or gcm_can_compute_global_sum
) indicating it has this ability
.false.
and MARBL is configured in a way that requires global sums, it will report an errorglobal_sum
an option in the upcoming marbl_saved_state_type
; by default, we would not sum saved state fields but to get a global sum you add a field to saved state and then request the summarbl_surface_forcing_output_type
is currently hardcoded to only output flux_o2
and flux_co2
; this needs to be generalized (and totalChl
added to the type). Adding totalChl
will allow us to move ecosys_driver_compute_totalChl
into MARBL.
It would be great if we could come up with a more general / robust way to pass namelists from the GCM to MARBL.
We had copies of c0
, c1
, c2
, c1000
, and p5
in marbl_kinds_mod.F90
, and I am in the process of moving them to a new marbl_constants_mod.F90
module, but it appears they are also stored in marbl_parms.F90
Obviously these only need to be stored in one place, but which module to use should depend on what other constants / parameters we end up saving and where we stick the parameters (will there be a marbl_params_type
inside marbl_interface_class
?)
We want module XYZ
in XYZ.F90
(and module ABC_mod
in ABC_mod.F90
). I'm anticipating the following file name changes when we are ready:
$ mv ecosys_diagnostics_mod.F90 marbl_diagnostics_mod.F90
$ mv ecosys_mod.F90 marbl_mod.F90
$ mv ecosys_restore.F90 ecosys_restore_mod.F90
$ mv marbl_kinds.F90 marbl_kinds_mod.F90
$ mv marbl_share.F90 marbl_share_mod.F90
We compute 100m vertical integrals in cases where we also compute the full-depth vertical integral and expect the latter to be 0 to round-off level... instead of returning these integrals as diagnostics, should MARBL check to make sure the full-depth integral is less than some threshold and error out if the threshold is exceeded?
FIXME comment preceding declaration of thermodynamic_coefficients_type
in marbl_co2calc_mod
move dic, ta, pt, sit into their own derived type
FIXME comment in marbl_mod::marbl_set_interior_forcing
:
need to pull particulate share out
of compute_particulate_terms!
marbl_mod::marbl_setup_local_tracers
currently initializes ALL tracers, but some are over-written by marbl_setup_local_zooplankton
and marbl_setup_local_autotrophs
The variable marbl_interior_share
is an intent(inout) in marbl_ciso_mod::marbl_ciso_set_interior_forcing
with a comment saying
FIXME - intent is inout due to DIC_loc
It looks like both DIC_loc
and DOC_loc
are set to 0 below the deepest ocean layer; maybe we can ensure these variables are prior to calling the routine? Or only update values for layers 1:column_kmt
?
marbl_restore_mod::init
should verify that the number of non-empty elements of restore_short_names
and restore_filenames
are the same, otherwise we are either requesting to restore a tracer without providing a climatology or providing a climatology without associating it with a specific tracer.
In some places CISO tracers are indexed from 1:ecosys_ciso_tracer_cnt
and in others they are ecosys_tracer_cnt+1:ecosys_tracer_cnt+ecosys_ciso_tracer_cnt
; we want to use the latter in all instances and rely on the ciso_on
flag to determine whether to do CISO computations or not.
In marbl_ciso_mod::marbl_ciso_set_interior_forcing
the call to marbl_store_diagnostics_ciso_interior
is preceded by the FIXME comment
the following arguments need to be grouped into a derived type
We should remove dz
from marbl_domain_type
and all layer depth references should use delta_z
(POP has a 1D array for using the same levels throughout the model and a 3D array for partial bottom cells; MARBL does not need to differentiate between these two.)
Currently, the GCM reads four MARBL namelists (ecosys_nml
, ecosys_ciso_nml
, ecosys_parms_nml
, and ecosys_restore_nml
) from a file and passes them as a string to MARBL. MARBL sets default values, then updates those values based on what the GCM passes.
Desired behavior: MARBL then writes the entire namelist to status log
Current behavior: MARBL writes the values set in the namelist file to status log
How do the GCMs produce namelists to be sent to MARBL? Does it make sense to have a build-namelist utility in MARBL? One option would be to produce XML files for definitions / default values as is done in CESM / ACME components.
As discussed at the Feb 29 meeting:
tracer_restore
function should be renamed compute_interior_restore
inv_tau
use the same interpolation routine as the diagnostics (which interpolate to find saturation depth for calcite and aragonite)?marbl_utils_mod.F90
)Note that work towards these issues should keep the big picture of Issue #14 in mind; it may not make sense to do the last task in this list (tracer index look-up routine) since it may not be needed once the interior restore process is more generalized. I think it will prove to be useful, though, since we still need to turn elements in the restore_short_names
namelist array into integers corresponding to the correct tracer index.
We discussed today the idea of a marbl_init_type
datatype, that would be set in marbl_instance%init()
and referenced in marbl_instance%set_interior()
and marbl_instance%set_surface_forcing()
. It would definitely contain ciso_on
, which is currently part of the driver because much of the if (ciso_on) then
logic is still in the driver rather than pushed into the subroutines in marbl_interface_class
. So ideally ciso_on
will be part of the marbl_interface_class
(which will involve moving it from ecosys_driver_nml
to ecosys_nml
).
Before we can do that we need to clean up the driver... which will be difficult due to the indexing scheme used by the tracers. Sometimes the CISO tracers are 1:ecosys_ciso_tracer_cnt
; other times they start the indexing at ecosys_tracer_cnt + 1
. Perhaps that move from the driver to MARBL should be a separate issue?
Labelling this a bug, but priority is somewhat low given the dependence on cleaning up the driver code.
Either move pi
computation to the initialization routine in marbl_ciso_mod
or save it in whatever module is still standing when #11 is resolved.
I have a sandbox on yellowstone where I can build libmarbl.a
, but before bringing it in to this repository we need to decide what the directory structure will look like. CVMix
is loosely based off the structure of HOMME
-- there is a src/shared/
directory containing everything needed to build libcvmix.a
, and the rest of src/
(as well as src/driver/
) is used to build the stand-alone driver / test-suite. Are we going to have any stand-alone tests in MARBL? As a straw-man, I'll propose the following structure:
./
-- Contains a README
file
src/
-- All Fortran code needed to build libmarbl.a
as well as dependency-generator tool and a Makefile
. I picture this Makefile requiring the user to pass in the compiler (as well as any compiler flags) explicitly... something like
$ make FC=gfortran FCFLAGS="-O2 -ffree-form"
driver/
-- If desired, standalone driver code could go here (along with its own Makefile
which would execute make -f ../src/Makefile
as part of the build)
I'd also like bld-test/
with a Makefile
that lets us use different compilers with simpler targets, e.g.
$ make intel
$ make gnu
These targets would execute make -f ../src/Makefile
with pre-determined FC
/ FCFLAGS
combinations to allow for easy / automated testing.
type(marbl_interface_class) :: marbl
to
type(marbl_interface_class), dimension(max_blocks_clinic) :: marbl
Currentl vflux_flag
resides in the driver and is set as follows:
vflux_flag(:) = .false.
vflux_flag(dic_ind) = .true.
vflux_flag(alk_ind) = .true.
vflux_flag(dic_alt_co2_ind) = .true.
But maybe we just want the driver to tell MARBL if it is using a virtual salt flux and then MARBL can set these flags accordingly? Where does this fall on the science vs infrastructure spectrum?
Several routines in marbl_interface_types
print error messages directly to stdout and have comments about needing to trigger an abort... these should all be using marbl_log_error
, which means they need to be passed a marbl_log_type
The computation of pChl_subcol, in marbl_compute_autotroph_photosynthesis, is
! GD 98 Chl. synth. term
pChl_subcol = autotrophs(auto_ind)%thetaN_max * PCphoto_subcol / &
(autotrophs(auto_ind)%alphaPI * thetaC * PAR_avg(subcol_ind))
The denominator can underflow to zero, even if the individual terms are positive. This happened in a coupled run using cesm_pop_2_1_20160222 (before MARBL was separated out). In this run, PAR_avg was less than 10^-300, in an individual sub-colum at a single grid point, but it was positive.
A work-around is to set PAR%interface to zero if it is below a threshold, after it is computed and before it is used to set PAR%avg. This is in marbl_compute_photosynthetically_available_radiation.
There are two FIXMEs related to marbl_interior_forcing_input_type
. One is in the type declaration:
FIXME: better names for PAR_col_frac and surf_shortwave
and one is in the variable declaration in marbl_interface.F90
:
FIXME: make this a 3d real array with indices
I'm not sure if this data type is one that we want to generalize, or if hard-coded variable names are okay, or if we just want a flat 3d array... but we definitely want to update it somehow.
Once we have merged #48 we should add a consistency_check()
routine to marbl_tracer_index_type
to ensure the indices of all desired tracers are non-zero
There is a FIXME comment in marbl_mod
that reads:
new marbl timers need to be implemented to turn
on timers here around this subroutine call
(subroutine call in question is marbl_compute_carbonate_chemistry
). Do we want MARBL to be able to provide timing information? If so, how do we incorporate that?
ecosys_ind_begin = 1
and ecosys_ind_end = ecosys_tracer_cnt
because we call set_tracer_indices
with 'ECOSYS'
before we call it with 'CISO'
. Throughout MARBL we are alternating between declaring vectors with dimension(ecosys_tracer_cnt)
and dimension(ecosys_ind_begin:ecosys_ind_end)
. We need to be consistent, but it is unclear which is preferable.
Right now we hard-code three parameters needed to pass namelists from GCM to MARBL:
marbl_nl_in_size is the number of characters in the entire namelist file
marbl_nl_cnt is the number of distinct namelists in the file
marbl_nl_buffer_size is the number of characters in the largest namelist
It would be great if there was a way to configure these values at run-time, though we need a smart way to do it. Maybe let the GCM determine them and pass values to MARBL during init?
Three similar FIXME comments in marbl_namelist_mod::marbl_nl_split_string
:
add error checking in acse
(i+1-old_pos) > marbl_nl_buffer_size
add error checking in case first character is not '&'
and in marbl_namelist_mod::marbl_namelist
:
add error checking in case &nl_name is not found
We should separate this into doc and docr
When building CESM with the gnu compiler, compsets that use the ecosystem ocean tracer module produce the following runtime error:
(passive_tracer_tools:read_field_3D) reading alk_riv_flux from /glade/p/cesmdata/cseg/inputdata/ocn/pop/gx1v6/forcing/river_nutrients_GNEWS2000_gx1v6.nc
(passive_tracer_tools:read_field_3D) reading doc_riv_flux from /glade/p/cesmdata/cseg/inputdata/ocn/pop/gx1v6/forcing/river_nutrients_GNEWS2000_gx1v6.nc
(tavg_requested) id = 0
(tavg_requested) FATAL ERROR: invalid tavg id
------------------------------------------------------------------------
POP aborting...
FATAL ERROR: invalid tavg id
------------------------------------------------------------------------
The error does not crop up when using the Intel or NAG compilers.
Jim Edwards suggests creating a timing module (marbl_timing_mod.F90) and using #ifdef
wrappers to support GPTL if requested at build time (otherwise we can fall back to walltime or something). This is already done (in a way) in POP & CIME; POP uses CCSMCOUPLED
in source/timers.F90
to trigger calls to cime/share/timing/perf_mod.F90
which is just a wrapper for GPTL. My first thought is to cut out the middle man and call GPTL directly if it is available.
This is not currently a high priority, but I would like to finish it sometime this summer... I believe it'll be useful to have #16 finished, because that bugfix will also provide the framework for a stand-alone driver and we'll be able to do some initial testing outside of CESM before trying to tie the MARBL timers in to POP.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.