-
Notifications
You must be signed in to change notification settings - Fork 55
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
Support different I/O types for history and restart output #2754
Support different I/O types for history and restart output #2754
Conversation
if int(0 if is_mpich_gpu_enabled is None else is_mpich_gpu_enabled) == 1: | ||
logger.info("Resetting support for GPU in MPICH/MPI library (since its not used by the tool)") | ||
os.environ['MPICH_GPU_SUPPORT_ENABLED'] = str(0); | ||
|
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.
A CIME config change.
This change is not in E3SM master yet, but we will be adding this change there soon
|
||
<environment_variables compiler="crayclang-scream" mpilib="mpich"> | ||
<env name="ADIOS2_ROOT">$SHELL{if [ -z "$ADIOS2_ROOT" ]; then echo /lustre/orion/cli115/world-shared/frontier/3rdparty/adios2/2.9.1/cray-mpich-8.1.26/crayclang-scream-14.0.0; else echo "$ADIOS2_ROOT"; fi}</env> | ||
</environment_variables> |
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.
A CIME config change
This change also needs to be ported to E3SM master too
<job name="case.st_archive"> | ||
<template>template.st_archive</template> | ||
<!-- If DOUT_S is true and case.run (or case.test) exits successfully then run st_archive--> | ||
<dependency>(case.run and case.post_run_io) or case.test</dependency> | ||
<!--dependency>(case.run and case.post_run_io) or case.test</dependency--> | ||
<dependency>case.run or case.test</dependency> |
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.
A CIME config change
This is a SCREAM repo only change for now. This change disables the ADIOS to NetCDF conversion job from running after the E3SM job completes.
We will need to add a way to disable it via XML changes in E3SM.
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.
This looks great! I have a couple of comments, but nothing too important.
@@ -350,7 +350,13 @@ void AtmosphereInput::finalize() | |||
/* ---------------------------------------------------------- */ | |||
void AtmosphereInput::init_scorpio_structures() | |||
{ | |||
scorpio::register_file(m_filename,scorpio::Read); | |||
int iotype = scorpio::str2iotype("default"); | |||
if(m_params.isParameter("iotype")){ |
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.
If you want, you can be more concise:
if(m_params.isParameter("iotype")){ | |
auto iotype_str = m_params.get<std::string>("iotype","default"); | |
int iotype = scorpio::str2iotype(iotype_str); |
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.
Thanks, your suggestion looks better/concise. I will make that change.
@@ -2,6 +2,7 @@ | |||
#define SCREAM_SCORPIO_INPUT_HPP | |||
|
|||
#include "share/io/scream_scorpio_interface.hpp" | |||
#include "share/io/scream_io_utils.hpp" |
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 don't think this include is really needed. If it's needed in the cpp, you can just include it in the cpp.
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.
ok, will remove it (I think it must have been added while I was modifying the code)
@@ -94,8 +94,8 @@ void eam_pio_finalize() { | |||
eam_pio_finalize_c2f(); | |||
} | |||
/* ----------------------------------------------------------------- */ | |||
void register_file(const std::string& filename, const FileMode mode) { | |||
register_file_c2f(filename.c_str(),mode); | |||
void register_file(const std::string& filename, const FileMode mode, const int iotype) { |
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.
Up to you, but since you often call this with iotype=IoType::DefaultIoType
, you could consider adding the default value in the fcn signature, so that you don't need to add , scorpio::IOType::DefaultIOType
to all register_file
calls..
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 think its a good idea to add the default value here (On the other hand forcing users to specify the args can prevent copy-paste errors :) ). I will make that change
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.
Yeah, it's not an obvious choice. As I said, up to you. If you think in the vast majority of cases the "default" value is fine, then adding defaults to the signature makes sense. But if the cost of using default when you shouldn't is very high, then we can just force users to always pass it.
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 will modify the code to use default args since there are many instances where the default io type is used (including tests etc)
460545c
to
40fb1d6
Compare
Status Flag 'Pull Request AutoTester' - GitHub reports Mergeable status = False |
40fb1d6
to
f720ff5
Compare
I have incorporated all changes suggested by @bartgol and rebased the branch with the latest master (to fix a merge conflict). |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
Using Repos:
Pull Request Author: jayeshkrishna |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: 1 or more Jobs FAILED Note: Testing will normally be attempted again in approx. 2 Hrs. If a change to the PR source branch occurs, the testing will be attempted again on next available autotester run. Pull Request Auto Testing has FAILED (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
SCREAM_PullRequest_Autotester_Mappy # 5041 FAILED (click to see last 100 lines of console output)
SCREAM_PullRequest_Autotester_Weaver # 5416 FAILED (click to see last 100 lines of console output)
|
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
Using Repos:
Pull Request Author: jayeshkrishna |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: 1 or more Jobs FAILED Note: Testing will normally be attempted again in approx. 2 Hrs. If a change to the PR source branch occurs, the testing will be attempted again on next available autotester run. Pull Request Auto Testing has FAILED (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
SCREAM_PullRequest_Autotester_Mappy # 5043 FAILED (click to see last 100 lines of console output)
SCREAM_PullRequest_Autotester_Weaver # 5418 FAILED (click to see last 100 lines of console output)
|
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
Using Repos:
Pull Request Author: jayeshkrishna |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: 1 or more Jobs FAILED Note: Testing will normally be attempted again in approx. 2 Hrs. If a change to the PR source branch occurs, the testing will be attempted again on next available autotester run. Pull Request Auto Testing has FAILED (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
SCREAM_PullRequest_Autotester_Mappy # 5045 FAILED (click to see last 100 lines of console output)
SCREAM_PullRequest_Autotester_Weaver # 5420 FAILED (click to see last 100 lines of console output)
|
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
Using Repos:
Pull Request Author: jayeshkrishna |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: 1 or more Jobs FAILED Note: Testing will normally be attempted again in approx. 2 Hrs. If a change to the PR source branch occurs, the testing will be attempted again on next available autotester run. Pull Request Auto Testing has FAILED (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
SCREAM_PullRequest_Autotester_Mappy # 5075 FAILED (click to see last 100 lines of console output)
SCREAM_PullRequest_Autotester_Weaver # 5444 FAILED (click to see last 100 lines of console output)
|
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
Using Repos:
Pull Request Author: jayeshkrishna |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: 1 or more Jobs FAILED Note: Testing will normally be attempted again in approx. 2 Hrs. If a change to the PR source branch occurs, the testing will be attempted again on next available autotester run. Pull Request Auto Testing has FAILED (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
SCREAM_PullRequest_Autotester_Mappy # 5077 FAILED (click to see last 100 lines of console output)
SCREAM_PullRequest_Autotester_Weaver # 5446 FAILED (click to see last 100 lines of console output)
|
Status Flag 'Pull Request AutoTester' - GitHub reports Mergeable status = False |
603fc9e
to
7c6106e
Compare
Status Flag 'Pull Request AutoTester' - Jenkins Testing: 1 or more Jobs FAILED Note: Testing will normally be attempted again in approx. 2 Hrs. If a change to the PR source branch occurs, the testing will be attempted again on next available autotester run. Pull Request Auto Testing has FAILED (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
SCREAM_PullRequest_Autotester_Mappy # 5199 FAILED (click to see last 100 lines of console output)
SCREAM_PullRequest_Autotester_Weaver # 5538 PASSED (click to see last 100 lines of console output)
|
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
Using Repos:
Pull Request Author: jayeshkrishna |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: 1 or more Jobs FAILED Note: Testing will normally be attempted again in approx. 2 Hrs. If a change to the PR source branch occurs, the testing will be attempted again on next available autotester run. Pull Request Auto Testing has FAILED (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
SCREAM_PullRequest_Autotester_Mappy # 5202 FAILED (click to see last 100 lines of console output)
SCREAM_PullRequest_Autotester_Weaver # 5541 PASSED (click to see last 100 lines of console output)
|
@jayeshkrishna there was a problem with the jenkins job that blessed the AT baselines (it still used gnu9 as compiler), so it didn't bless the baselines. I fixed it and I'm re-running it. I also added the RETEST label so that the testing will be triggered again. |
Status Flag 'Pull Request AutoTester' - User Requested Retest - Label AT: RETEST will be reset after testing. |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
Using Repos:
Pull Request Author: jayeshkrishna |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: 1 or more Jobs FAILED Note: Testing will normally be attempted again in approx. 2 Hrs. If a change to the PR source branch occurs, the testing will be attempted again on next available autotester run. Pull Request Auto Testing has FAILED (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
SCREAM_PullRequest_Autotester_Mappy # 5210 FAILED (click to see last 100 lines of console output)
SCREAM_PullRequest_Autotester_Weaver # 5549 PASSED (click to see last 100 lines of console output)
|
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
Using Repos:
Pull Request Author: jayeshkrishna |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
|
Status Flag 'Pre-Merge Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
All Jobs Finished; status = PASSED, target_sha=d60caf628c909505442693c42f0a291bbbd17c81, However Inspection must be performed before merge can occur... |
Status Flag 'Pull Request AutoTester' - AutoMerge IS ENABLED, but the Label AT: AUTOMERGE is not set. Either set Label AT: AUTOMERGE or manually merge the PR... |
@jayeshkrishna , with the upstream merge in place and this PR passing the AT and approved is it safe to say this is ready to go in? |
Yes, this PR should be ready to merge. |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
Using Repos:
Pull Request Author: jayeshkrishna |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
|
Status Flag 'Pull Request AutoTester' - AutoMerge IS ENABLED, but the Label AT: AUTOMERGE is not set. Either set Label AT: AUTOMERGE or manually merge the PR... |
I added automerge, since I believe there was no reason to hold off the merge. The AT will test once more (since master has changed since last PASS), then will merge. |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
Using Repos:
Pull Request Author: jayeshkrishna |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
|
Adding support for using different I/O types (pnetcdf, adios, netcdf) for history and restart output.
The I/O type for restart output is picked up from CIME for E3SM runs, while the I/O type for the history output is controlled via a new YAML key "iotype" in the output YAML configuration file.
This PR also updates SCORPIO to 1.6.2 and CIME to cime6.0.226