Reduce redundancy; make interfaces more similar between tools; focusing here on fsurdat_modifier and subset_data #1714
Conversation
Mostly I moved text from the tool's docstring to the README and made a note of that in the tool's docstring. Also I included brief explanations of the differences between: - this tool and modify_singlept_site_neon.py - this tool and subset_data
|
Update README: |
Fix accumulation variables when changing model time step Accumulation variables (e.g., 1-day or 10-day averages) were writing and reading their accumulation period (expressed in time steps) to the restart file. This caused incorrect behavior when changing the model time step relative to what was used to create the initial conditions file (typically a 30-minute time step). So, for example, if you are using a 15-minute time step with an initial conditions file that originated from a run with a 30-minute time step (at some point in its history), then an average that was supposed to be 10-day instead becomes 5-day; an average that was supposed to be 1-day becomes 12-hour, etc. (The issue is that the number of time steps in the averaging period was staying fixed rather than the actual amount of time staying fixed.) For our out-of-the-box initial conditions files, this only impacts runs that use something other than a 30-minute time step. Typically this situation arises in configurations with an active atmospheric model that is running at higher resolution than approximately 1 degree. It appears that the biggest impacts are on VOC emissions and in BGC runs; we expect the impact to be small (but still non-zero) in prescribed phenology (SP) runs that don't use VOC emissions. This tag fixes this issue by no longer writing or reading accumulation variables' PERIOD to / from the restart file: this isn't actually needed on the restart file. See some discussion in ESCOMP#1789 for more details, and see ESCOMP#1802 (comment) for some discussion of outstanding weirdness that can result for accumulation variables when changing the model time step. The summary of that comment is: There could be some weirdness at the start of a run, but at least for a startup or hybrid run, that weirdness should work itself out within about the first averaging period. A branch or restart run could have some longer-term potential weirdness, so for now I think we should recommend that people NOT change the time step on a branch or restart run. With (significant?) additional work, we could probably avoid this additional weirdness, but my feeling is that it isn't worth the effort right now. In any case, I feel like my proposed fix will bring things much closer to being correct than they currently are when changing the time step. Resolved conflict: python/ctsm/modify_fsurdat/fsurdat_modifier.py
slevis-lmwg
added
the
PR status: ready
|
As with the other PR make sure 51c102c is added to the .git-blame-ignore-revs file. |
|
python testing PASS |
Resolved conflicts and updated two files: doc/ChangeLog doc/ChangeSum
|
Python and pymod testing PASS On cheyenne and izumi I executed: @ekluzek I consider this PR ready to merge. |
|
OK, awesome, you've done everything I see. I'm not able to tie this off today, but should be Wednesday night or Thursday. |
|
@slevisconsulting on the pylint issue, make sure you setup the ctsm_py environment and activate it, and that the error is seen under that environment. pylint is very sensitive to the exact version being used. If we see it in that environment we should fix it. |
|
@ekluzek aren't you on PTO...? |
Good news @ekluzek. As you expected, once I executed Thanks! |
Conflicts: doc/ChangeLog doc/ChangeSum
Description of changes
Details in #1662
Specific notes
Starting by replacing zero_nonveg option with include_nonveg
Contributors other than yourself, if any:
@ekluzek @negin513 @adrifoster
CTSM Issues Fixed (include github issue #):
#1662
Are answers expected to change (and if so in what way)?
No
Any User Interface Changes (namelist or namelist defaults changes)?
Making interfaces more similar between tools, e.g. replacing zero_nonveg with include_nonveg
Testing performed, if any:
This far...
make lint: PASS
make test: OK (pre-existing errors in
test_buildSetup_userDefinedMachine_allInfoandtest_buildSetup_userDefinedMachine_minimalInfo