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.

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

Move PIO type control to individual streams for MPAS-O and MPAS-SEAICE #1456

Merged
merged 3 commits into from
May 16, 2017

Conversation

jonbob
Copy link
Contributor

@jonbob jonbob commented Apr 25, 2017

This PR moves control of PIO_TYPENAME to individual files by including settings for "io_type" for each file in the mpas-o and mpas-seaice streams. For high-res grids, where some 3D fields require special handling, specific files are given the following setting:
io_type="pnetcdf,cdf5"
Otherwise, all streams are assigned the default PIO_TYPENAME for the machine being run on. The drivers are also modified to remove the setting of a default io_type, which would overwrite the specific stream settings.

Tested for correct streams output via:
-compset GMPAS-IAF -mach edison -compiler intel -res T62_oEC60to30v3
-compset GMPAS-IAF -mach edison -compiler intel -res T62_oRRS18to6v3

Need throughput testing at high-res!

[BFB]

@jonbob jonbob self-assigned this Apr 25, 2017
Copy link
Contributor

@mark-petersen mark-petersen left a comment

Choose a reason for hiding this comment

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

This looks like just what we need. I inspected visually, did not test it myself. But the lines changed are the same as what I used to test the io_type="pnetcdf,cdf5" on edison by directly changing the streams file. Thanks!

Copy link
Contributor

@vanroekel vanroekel left a comment

Choose a reason for hiding this comment

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

@jonbob Thanks so much for making this automated! I had just a few needed changes. I might just suggest making all timeSeriesStats cdf5 in case someone puts normal velocity in there. It is in timeSeriesStatsMonthly by default, so the io_type should be changed for that one at least.

@@ -379,6 +397,7 @@ if ( -e "$CASEROOT/SourceMods/src.mpaso/$STREAM_NAME" ) {
print $stream_file '' . "\n";
print $stream_file '<stream name="globalStatsOutput"' . "\n";
print $stream_file ' type="output"' . "\n";
print $stream_file ' io_type="' . "$OCN_PIO_TYPENAME" . '"' . "\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could use io_type="netcdf" here too

Copy link
Contributor

Choose a reason for hiding this comment

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

testing on titan suggests netcdf here is much faster (1.4 max seconds per process with pnetcdf vs. .06 max seconds with netcdf)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vanroekel - our idea has been to overwrite the io_type for streams we know need cdf5, but otherwise fall back to the machine defaults. So I'll make the changes to the streams you've specified, and check that titan's default is netcdf. If it's not, we'll have to ask the titan POC to make that change... Thanks for the careful review!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vanroekel - but you're also correct that we discussed using "netcdf" for streams we know are small and will be negatively impacted by possible using pnetcdf. @mark-petersen , what do you think about this one?

Copy link
Contributor

Choose a reason for hiding this comment

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

wolf was really slow with pnetcdf, so we set all defaults to netcdf. Are there any examples of the opposite - machines that are very slow for netcdf for small files? I've never heard that, so it seems like it would be safe to set the default to netcdf for all small files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mark-petersen - Agreed. I think this is a safer approach and more likely to be stable over the range of machines. I'll consider this PR ready to go as it currently is then...

@@ -398,6 +417,7 @@ if ( -e "$CASEROOT/SourceMods/src.mpaso/$STREAM_NAME" ) {
print $stream_file '' . "\n";
print $stream_file '<stream name="surfaceAreaWeightedAveragesOutput"' . "\n";
print $stream_file ' type="output"' . "\n";
print $stream_file ' io_type="' . "$OCN_PIO_TYPENAME" . '"' . "\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above.

@@ -655,6 +688,7 @@ if ( -e "$CASEROOT/SourceMods/src.mpaso/$STREAM_NAME" ) {
print $stream_file '' . "\n";
print $stream_file '<stream name="timeSeriesStatsMonthlyOutput"' . "\n";
print $stream_file ' type="output"' . "\n";
print $stream_file ' io_type="' . "$OCN_PIO_TYPENAME" . '"' . "\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

This definitely needs cdf5, due to normal velocity being in here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that in this PR any stream with normalVelocity should be cdf5 for high rez. Thanks @vanroekel for thinking of that.

On Thursday I'll propose making a separate monthly time-averaged stream for edge variables, but that is yet to be agreed upon and tested.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and this one

@@ -743,6 +780,7 @@ if ( -e "$CASEROOT/SourceMods/src.mpaso/$STREAM_NAME" ) {
print $stream_file '' . "\n";
print $stream_file '<stream name="timeSeriesStatsMonthlyRestart"' . "\n";
print $stream_file ' type="input;output"' . "\n";
print $stream_file ' io_type="' . "$OCN_PIO_TYPENAME" . '"' . "\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

This definitely also needs cdf5. Normal velocity is in here, which violates constraints.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and one more...

@@ -558,6 +587,7 @@ if ( -e "$CASEROOT/SourceMods/src.mpaso/$STREAM_NAME" ) {
print $stream_file '' . "\n";
print $stream_file '<stream name="eliassenPalmOutput"' . "\n";
print $stream_file ' type="output"' . "\n";
print $stream_file ' io_type="' . "$OCN_PIO_TYPENAME" . '"' . "\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this will need cdf5 too, it is a very heavy stream.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, changed this one

@vanroekel
Copy link
Contributor

Thanks @jonbob, looks good to me!

@mark-petersen
Copy link
Contributor

This final version looks good to me too.

@@ -279,6 +295,7 @@ if ( -e "$CASEROOT/SourceMods/src.mpaso/$STREAM_NAME" ) {
print $stream_file '' . "\n";
print $stream_file '<stream name="output"' . "\n";
print $stream_file ' type="output"' . "\n";
print $stream_file ' io_type="' . "$OCN_PIO_TYPENAME" . '"' . "\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

@jonbob testing in the 18to6 spinup I note that this stream needs pnetcdf,cdf5 as well. normalVelocity is in this stream, which violate size constraints.

jonbob added a commit that referenced this pull request May 15, 2017
This PR moves control of PIO_TYPENAME to individual files by including settings
for "io_type" for each file in the mpas-o and mpas-seaice streams.  For high-res
grids, where some 3D fields require special handling, specific files are given
the following setting:
     io_type="pnetcdf,cdf5"
Otherwise, all streams are assigned the default PIO_TYPENAME for the machine
being run on.  The drivers are also modified to remove the setting of a default
io_type, which would overwrite the specific stream settings.

Tested for correct streams output via:
-compset GMPAS-IAF -mach edison -compiler intel -res T62_oEC60to30v3
-compset GMPAS-IAF -mach edison -compiler intel -res T62_oRRS18to6v3

[NML]
[BFB]
@jonbob
Copy link
Contributor Author

jonbob commented May 15, 2017

Merged to next

@jonbob jonbob merged commit 51ea5bd into master May 16, 2017
jonbob added a commit that referenced this pull request May 16, 2017
This PR moves control of PIO_TYPENAME to individual files by including settings
for "io_type" for each file in the mpas-o and mpas-seaice streams.  For high-res
grids, where some 3D fields require special handling, specific files are given
the following setting:
     io_type="pnetcdf,cdf5"
Otherwise, all streams are assigned the default PIO_TYPENAME for the machine
being run on.  The drivers are also modified to remove the setting of a default
io_type, which would overwrite the specific stream settings.

Tested for correct streams output via:
-compset GMPAS-IAF -mach edison -compiler intel -res T62_oEC60to30v3
-compset GMPAS-IAF -mach edison -compiler intel -res T62_oRRS18to6v3

[NML]
[BFB]
@jonbob
Copy link
Contributor Author

jonbob commented May 16, 2017

Merged to master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants