-
Notifications
You must be signed in to change notification settings - Fork 25
2018 meeting notes
Most of the discussion was looking several steps ahead in my work to create a test for set_surface_forcing() and set_interior_forcing(). Some key points:
- We should do a better job organizing the
tests/directory:-
init-twiceis a unit test, in the sense that it really testsmarbl_instance%shutdown()to ensure we are deallocating everything that has been allocated. - Once
set_forcing(which should be renamedcompute_tendenciesor something) is finished, we don't need separate a regression test forinit(we can print the MARBL log in the full test and then changes in init will result in differences in standard out) - All the
requested_*tests would be better labelled asexamplesinstead oftests; would this require a completely different executable?
-
- We need to improve the build system to handle the need for netCDF
- require netCDF for test directory (but maybe not for examples)?
- require users to build via python scripts (add
FROM_PYTHON=FALSEto Makefile, overwrite from python scripts, abort unlessFROM_PYTHON == TRUE) -
nc-configmay not be consistent enough across machines to use it to get include / linking flags (nc-config --flibsdoes not give reasonable results when using homebrew to install on a Mac; other options did not play nicely on hobart)
- If we rely on python for the build, could also use config files (read by python, passed to
make) to store machine-specific settings (and also state of current build)
Ahead of the MARBL v1 release, I should sort remaining MARBL 1.0.0 issues into "API changing" and "not API changing"; it may make sense to push some of the non-API-changing issues back until after the release and focus on finalizing the interface. Speaking of API changes, we should probably include both short name and long name in the forcing metadata type (instead of varname, which is currently what we provide to the GCM).
For the jupyter notebook generating forcing data sets, Matt recommended using xarray instead of netCDF4... also, besides the southern ocean data point a location in the eastern equatorial Pacific would be interesting. lat_in and lon_in should be lists, but each individual column should be written to its own netcdf file.
Jessica's issues will be addressed, and some in-ticket comments were made but no clear priority decided. Currently the python code processing templated diagnostics in YAML assumes one template replacement per variable, so something like {auto}_graze_{zoo}_to_{zoo} would require python overhaul. We also discussed idea of having multiple POC pools, which would be a huge undertaking because of the effect it would have on routing.
For the set_forcing test, besides outputting tracer tendencies and surface forcing values, I'll also include diagnostics. For now I'll just output all computed diagnostics, though it might be nice in the future to have an optional argument to the test akin to --input-file to allow users to customize the diagnostic output.
Now that the low-hanging fruit has been dealt with, we talked about how to move on to the next big issue -- improving the functionality of marbl_domain_type. We decided that it made the most sense to do the following:
- Create a stand-alone test for
set_surface_forcingandset_interior_forcing - Use this new test to make sure we only loop from 1 to
kmt(lots of loops currently run tokm, which isn't necessary) - Once all loops are the right size, we can work on the interface to allow GCMs where
k = 1is the bottom of the column rather than the surface.
More talk on #1 (aborting if mass balance fails). This would have saved Jessica a lot of time when she stumbled upon bug #224; current plan would be to introduce an assert_near_zero() function. Keith would like to hard-code in the tolerance, which will require looking at current POP output to determine how big each of the mass balance terms can be. This is preferable to determining tolerance on the fly, because the latter might mask some changes we would like to see. (Also, we can't necessarily use the 100m integral as a basis for determining the tolerance because if the column is <100 m deep then the mass balance = the 100 m integral).
Testing the diagnostics with Jessica's size-structured set-up highlighted a bug with how logical settings are handled - e.g. length of tracer_restore_vars depends on whether ciso_on = ".true." but some users would set ciso_on = "T" instead. (Solution is to convert all acceptable Fortran logical values to either ".true." or ".false." so the comparison is always valid.) We also discussed combining POP and MARBL diagnostics into a single file (ecosys_diagnostics).
My plan once the diagnostics work is on the trunk will be to go back and address several tickets that seemingly have quick fixes; this should coincide nicely with everyone else being at Ocean Sciences.
The majority of time in this meeting was spent digging into the weeds of the YAML / python setup for defining diagnostics. Lots of good suggestions, such as better implementation of templating (things like ((autotroph_sname)) for autotroph short name) and not introducing an additional level of dictionaries just to handle per-autotroph and per-tracer diagnostics.
PGI continues to introduce compiler bugs that prevent MARBL from running (which is actually an improvement from the days where PGI compiler bugs prevented MARBL from building). Need to figure out if dropping PGI support is okay with CESM / E3SM, or if some nominal testing needs to continue as PGI updates its compiler.
Walking through the updates to default_settings.yaml (to include tracer short-names instead of just total count) raised some internal inconsistencies -- marbl_interface_private_types.F90 refers to tracers in all lower-case (po4 and caco3) while marbl_init_mod.F90 actually sets up the tracer metadata using proper atomic case (PO4 and CaCO3). The YAML file will use the marbl_init_mod.F90 case, and hopefully we can auto-generate both marbl_interface_private_types.F90 and marbl_init_mod.F90 based on the YAML so that they will be consistent as well.
We prioritized current open tickets to determine what we want in the CESM2.0 sub-milestone of the MARBL 1.0.0 milestone.
Discussion on how to fix the bug in interior restoring, which should be on master by the next meeting. This fix will definitely go into CESM 2.0, but it does not need to be in the next beta tag... so the fix will get merged onto marbl_dev immediately but not pushed to the trunk just yet.
Not much MARBL-specific conversation, just walked through NCAR/manage_externals and how it will be used in CESM tags following cesm2_0_beta08 (and also in POP starting in whatever trunk tag goes into cesm2_0_beta09)