CESM-Development / water-isotopes

bug tracker for cesm water isotopes
Other
0 stars 0 forks source link

review mosart-wiso branch #15

Open bandre-ucar opened 7 years ago

bandre-ucar commented 7 years ago

Summary of Issue:

Review the mosart-wiso branch with passive tracers.

bandre-ucar commented 7 years ago

Email from Tony Craig 2016-08-18:

Anthony.P.Craig <anthony.p.craig@gmail.com>
Aug 18
Reply to HongYi, David, me, Mariana 

I have had a quick look at the differences in the geotrace branch and the mosart1_0_17
version of the model.  Nothing stands out as in terms of major errors, but there are several things 
that should be improved.  This is a software review of the code, I have no idea whether the code
runs or produces reasonable science.

- nliq and nfrz as well as the other tracer indices are defined in a few places, the rof_comp_*,
and rof_cpl_indices.  I think it would be good to unify that implementation and have the
indices defined only in rof_cpl_indices.

- we need to move away from "nt1", "nt2", ..., "nt8" in terms of hardwired arrays for use
in the history files.  this needs to be generalized to handle an arbitrary number of terms.
when we had 2 terms, it was OK (not great).  now that we have more terms and maybe
variable numbers of terms depending on run, it should be done properly.

- wiso_runoff seems to be a flag that is used to turn on the new tracers.  this flag is hardwired
into RtmVar.F90.  This needs to be a namelist!

- we need to make sure the code runs fine and is bit-for-bit with the new tracers off compared
to the trunk version.  I think this kind of testing is critical.  To that point, all the standard
things like exact restart, bit-for-bit on different pe counts, threading, etc have to be tested
if they haven't yet.

- we need to make sure we're clear how we're calling the mosart physics (euler) for each
tracer and how we're managing what physics are called inside mosart physics for each
tracer.  There is currently a flag called Tunit%euler_calc that defines which tracers euler is
called for.  That is true for everything except frozen ice.  We need to make sure that's
correct.  Remember that frozen ice is advected directly to the outlet.  So shouldn't the same
be for all the new tracers associated with the frozen variable?  This seems inconsistent.
Separately, there are now several new "if" tests inside the MOSART_physics code that checks
the tracer ID then either does something or not.  This seems all very risky.  The logic that
controls what physics is done on which tracers is not transparent AT ALL.  We need to come
up with a better way to control that.  Maybe we need to define some new arrays that control
which physics is called as a function of tracer and have those set at initialization (or something
like that).

- There are several "indentation" issues in MOSART_physics with the code changes.  That
code should be cleaned up.

Those are my thoughts for now.  Let me know if you have any questions.  And just to let you
know, I will be going on vacation on Saturday for two weeks and will be reading and replying 
to email, but will generally be on reduced hours until Sept 4.

thanks,

tony
bandre-ucar commented 7 years ago

Email from HongYi 2016-10-18:

Hi Ben,

I have checked my recent changes on MOSART-tracer into the NCAR repository. The tag https://svn-ccsm-models.cgd.ucar.edu/mosart/branch_tags/geotrace_tags/geotrace_n02_mosart1_0_18/ is for mass balance check with passive tracers. https://svn-ccsm-models.cgd.ucar.edu/mosart/branch_tags/geotrace_tags/geotrace_n03_mosart1_0_18/ is the tag with the passive tracers commented off, so could be coupled with a CLM-tracer branch/tag.

Please indicate how you would suggest we proceed.

Thanks,
HongYi