add Newton-Krylov solver related tavg vars to abio_dic_dic14 #36
Conversation
Testing: passes aux_pop on cheyenne/intel, compared to pop2_cesm2_1_rel_n10
expected NLCOMP & BASELINE failures for new test
some MEMCOMP failures
computation and inclusion of vars in tavg file controlled with nml var abio_dic_dic14_ltavg_NK
nml var enabled in new test ERS_Ld5_D.T62_g37.C.cheyenne_intel.pop-abio_dic_dic14_ltavg_NK
mv io_read_fallback_register_field call for 'ABIO_PH_SURF' outside of all_fields_exist_in_restfile conditional
BranchChangeLog
Outdated
| Tag Creator: klindsay | ||
| Developers: klindsay | ||
| Tag Date: 21 Jun 2020 | ||
| Tag Name: pop2/trunk_tags/cesm_pop_2_1_20190306 |
There was a problem hiding this comment.
You have Tag Name listed twice, and I think the trunk_tag name is incorrect
There was a problem hiding this comment.
extra Tag Name line removed
@mnlevy1981 , I haven't actually made the tag, assuming that this step is performed after the PR is merged.
Is that correct?
There was a problem hiding this comment.
@klindsay28 That's right -- I'll merge the branch and then create the tag. I can update the Tag Date when I do the merge (git merge --no-commit --no-ff will let me modify files the same way we modify files if there is a conflict in the merge). I guess I could've cleaned up the tag name as well...
| <!-- abio_dic_dic14 derived vars --> | ||
| <!---------------------------------> | ||
|
|
||
| <abio_dic_dic14_ltavg_NK>.false.</abio_dic_dic14_ltavg_NK> |
There was a problem hiding this comment.
It seems like this variable serves the same function as lecosys_tavg_all (it's not passed to the Fortran code, it only exists so build-namelist and the ocn.*.tavg.csh script know what output to include in tavg_contents. Given that, should the variable name be structured similarly? I'd expect labio_tavg_NK or labio_dic_dic14_tavg_NK.
There was a problem hiding this comment.
I prefer abio_dic_dic14_ltavg_NK, to keep with the convention of all abio_dic_dic14 related nml vars having the module name as a prefix.
I suggest that, at some point, we unify all ecosys related nml vars to have an ecosys_ prefix, including changing lecosys_tavg_all to ecosys_ltavg_all. I think that is well beyond the scope of this PR.
There was a problem hiding this comment.
I opened #37 to track this thread, as I agree it's beyond the scope of this PR. I'm on the fence about whether to actually address the issue given the pending retirement of POP in CESM, but that's a discussion for the new issue ticket :)
There was a problem hiding this comment.
Looks good now! @alperaltuntas do you want to look through the changes as well, or is it okay if I merge this?
Description of changes:
add tavg vars needed for Newton-Krylov solver to abio_dic_dic14
Move io_read_fallback_register_field call for 'ABIO_PH_SURF' outside of all_fields_exist_in_restfile conditional. This fallback should always be available, and the Newton-Krylov solver relies on it.
Testing:
Test case/suite: passes aux_pop on cheyenne/intel, compared to pop2_cesm2_1_rel_n10
Test status: bit for bit
new test ERS_Ld5_D.T62_g37.C.cheyenne_intel.pop-abio_dic_dic14_ltavg_NK added to aux_pop
Fixes: NA
User interface (namelist or namelist defaults) changes?
computation and inclusion of vars in tavg file controlled with nml var abio_dic_dic14_ltavg_NK
(nml var not written to pop_in, just passed to tavg_contents generating script)