Simplify snapshot checking reporting and functionality #269
Conversation
|
|
||
| ! Determine if physics_check should be run: | ||
| if (trim(ncdata_check) /= trim(unset_path_str)) then | ||
| call physics_check_data(ncdata_check, suite_names, data_frame, & | ||
| min_difference, min_relative_value) | ||
| if (do_ncdata_check) then |
There was a problem hiding this comment.
do we still want to include the check if ncdata_check is populated in order to proceed to the check? I see that there's logic in physics_check data to issue a warning ('WARNING: Namelist variable ncdata_check is UNSET. Model will run, but physics check data will not be printed') if it's not, but I can also imagine that warning would be confusing if you had no intention of running the check.
There was a problem hiding this comment.
That's a good question! I guess my opinion would be to just skip the call to physics_check_data entirely if we can avoid it, so I would prefer to leave this ncdata_check if-statement here as-is.
That being said, I am also not sure if the extra internal logic inside of physics_check_data you mentioned is really hurting anything either (beyond maybe a few wasted clock cycles). So that combined with the fact that I am not totally sure what the long-term plan for the physics_check_data routine will be (e.g. will it eventually be called elsewhere in SIMA? Removed entirely?) makes me lean towards just leaving everything as-is on that front for now as well. Of course if you or @cacraigucar feel strongly one way or the other please let me know. Thanks!
There was a problem hiding this comment.
I like avoiding writing out the WARNING message for runs where we are not performing checks (normal CAM-SIMA runs in the future?) I figure when we get to that stage, we can make sure that everything is running "best" for CAM-SIMA and make any required tweeks at that time. So I don't have a strong feeling one way or the other.
There was a problem hiding this comment.
Well, turns out I can't read. The if (trim(ncdata_check) /= trim(unset_path_str)) then is still there and I thought it was gone. So.. agreed and nevermind!
|
|
||
| ! Determine if physics_check should be run: | ||
| if (trim(ncdata_check) /= trim(unset_path_str)) then | ||
| call physics_check_data(ncdata_check, suite_names, data_frame, & | ||
| min_difference, min_relative_value) | ||
| if (do_ncdata_check) then |
There was a problem hiding this comment.
Well, turns out I can't read. The if (trim(ncdata_check) /= trim(unset_path_str)) then is still there and I thought it was gone. So.. agreed and nevermind!
This PR ensures that snapshot checks (e.g.
ncdata_check) are only run during actual model run steps (not initialize or finalize steps), and that there is no offset between the model timestep and the input or check file time.This PR also adds a new
current_timestep_numberregistry variable which is updated every time the model time step advances, as well as a new log print statement that indicates which model timestep CAM-SIMA is currently on.Fixes #268
Tests run:
Ran a physics testbed configuration with both Kessler and Held-Suarez physics for both a single timestep and for multiple timesteps (no differences found for any of the tests).
Also ran the official regression build tests on derecho for both GNU and Intel.