-
Notifications
You must be signed in to change notification settings - Fork 279
Regression test bug fixes for running on Jet or with Rocoto #981
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.
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
Changes from all commits
77c71b5
7e5dfe2
8bdac0c
ef6e857
d95c748
9b1d69f
3288868
2e439db
10193fd
9c37901
dedaf2e
321c5d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this and the similar change in
run_test.shnecessary? I ran RT with and without these changes and did not notice any difference in any of the log files.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless I'm mistaken, you need to redirect via tee to see logs in the job's own log file. Otherwise it only goes to the out and err files. There is similar code (but less reliable) in the non-Rocoto portions of the script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At some point we should remove all rocoto code, we do not use it and do not test it. For parallel execution we use ecflow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll retest this. I know I added this change in two locations. Let me check if I really need them in both locations. This may take a whole day due to queue wait times.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DusanJovic-NOAA - It is premature to remove Rocoto support since most people outside EMC and NCO have no experience with ecFlow. I can keep it working on Jet and Hera until that interferes with my official duties.
Edit: I meant to say no experience with ecFlow.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MinsukJi-NOAA I retested. Yes, the changes are necessary. This is half of the bug fix for the critical bug in the workflow: the job deleted the out and err files Rocoto created in its
#SBATCHlines, causing the rt_test.sh to conclude the test failed (because its grep failed). Now, Rocoto and the job_card each have their own log files, and this line ensures the logging goes to both.