Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Stop running 0th timestep #67

Merged
merged 3 commits into from
Nov 12, 2024
Merged

Stop running 0th timestep #67

merged 3 commits into from
Nov 12, 2024

Conversation

olyson
Copy link
Contributor

@olyson olyson commented Aug 1, 2023

Changes needed to stop running 0th timestep.
See CTSM issue ESCOMP/CTSM#925 and PR ESCOMP/CTSM#2084

jedwards4b
jedwards4b requested changes Dec 14, 2023
View reviewed changes
Copy link
Contributor

@jedwards4b jedwards4b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the change @olyson. Please remove the comments. They are forever preserved in github.

src/riverroute/RtmHistFile.F90 Outdated Show resolved Hide resolved
src/riverroute/RtmTimeManager.F90 Outdated Show resolved Hide resolved
@mvertens mvertens requested a review from jedwards4b January 12, 2024 10:49
@slevis-lmwg
Copy link
Contributor

@olyson I assigned you because you worked on this, not because i need something from you at this time.

@slevis-lmwg slevis-lmwg mentioned this pull request Nov 11, 2024
Merged
@slevis-lmwg
Merge tag 'mosart1.1.04' into zerothtstep
16bc164 
time in hist now equals the middle of the time_bounds

MOSART equivalent to CTSM work done in ESCOMP/CTSM#2838
Answers change only for the time variable.

slevis resolved conflicts:
src/riverroute/RtmHistFile.F90
src/riverroute/RtmTimeManager.F90
@slevis-lmwg
Copy link
Contributor

@olyson I have @jedwards4b approval here, but mosart's timemanager changed since you worked on this, such that the line
is_first_step = (nstep == 0)
is not there to remove anymore. From a quick look, I don't see other cases of nstep that need modification, but could you confirm?

@olyson
Copy link
Contributor Author

olyson commented Nov 12, 2024

I guess the function itself (is_first_step) has been removed since this PR was first issued, so that seems ok. I don't see any other instances of nstep that need modification.

@slevis-lmwg
Copy link
Contributor

slevis-lmwg commented Nov 12, 2024 •
edited
Loading

derecho
OK ./run_sys_tests -s mosart -c mosart1.1.04-ctsm5.3.009 -g mosart1.1.05-ctsm5.3.011

izumi
OK ./run_sys_tests -s mosart -c mosart1.1.04_ctsm5.3.009 -g mosart1.1.05_ctsm5.3.011

@slevis-lmwg slevis-lmwg merged commit 99b425e into ESCOMP:master Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jedwards4b jedwards4b jedwards4b approved these changes

Assignees

@olyson olyson

@slevis-lmwg slevis-lmwg

Labels
code cleanup enhancement high priority
Projects
LMWG: Near Term Priorities
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@olyson @slevis-lmwg @jedwards4b