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

Add TimeSeriesStatsRestartTest to global ocean #435

Closed
wants to merge 4 commits into from

Conversation

xylar
Copy link
Collaborator

@xylar xylar commented Oct 5, 2022

This merge adds a new TimeSeriesStatsRestartTest test case that makes sure the timeSeriesStats* analysis member
produces the same results with a full 8-hour run and with two 4-hour runs with a restart in between. Tests are available with or without analysis restart files (the former is the MPAS-Ocean standalone default, the latter is the behavior in E3SM).

These tests exist with the variables used in both daily and monthly averages, though they are not computed over so long a time interval for efficiency.

QU240 versions of these test cases have been added to the pr test suite for both daily and monthly output and both with and without analysis restarts.

The tests will fail with the current E3SM master. They pass with the bug fixes in E3SM-Project/E3SM#5218.

@xylar xylar added enhancement New feature or request python package DEPRECATED: PRs and Issues involving the python package (master branch) E3SM PR required labels Oct 5, 2022
@xylar xylar self-assigned this Oct 5, 2022
@xylar xylar force-pushed the add_time_daily_restart_test branch 2 times, most recently from 8692be7 to 5f238fa Compare October 5, 2022 18:52
Copy link
Collaborator

@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.

Tested with gnu debug, gnu optimized, and intel optimized using all daily and monthly timeSeriesStats tests and this PR for compass, and the code PR E3SM-Project/E3SM#5218.

I temporarily added a test to compass that could be added to this PR, if you think it would be useful @xylar

cat ./compass/ocean/suites/timeseriesstats.txt

ocean/global_ocean/QU240/mesh
ocean/global_ocean/QU240/PHC/init
ocean/global_ocean/QU240/PHC/daily_output_test
ocean/global_ocean/QU240/PHC/daily_analysis_restart
ocean/global_ocean/QU240/PHC/daily_model_restart
ocean/global_ocean/QU240/PHC/monthly_output_test
ocean/global_ocean/QU240/PHC/monthly_analysis_restart
ocean/global_ocean/QU240/PHC/monthly_model_restart

@xylar
Copy link
Collaborator Author

xylar commented Oct 5, 2022

@mark-petersen, thanks very much for testing! I added the suite as you suggested. To be consistent with other suite names, I used underscores in the name: time_series_stats. I hope that's okay with you.

@xylar
Copy link
Collaborator Author

xylar commented Oct 5, 2022

Testing

I ran the pr test suite on Anvil with Intel and IMPI using this branch and E3SM-Project/E3SM#5202 as a baseline, and this branch an E3SM-Project/E3SM#5218 for comparison. All tests ran successfully and most validation failures were as expected:

However, I saw validation failures in both suits for ocean_baroclinic_channel_10km_decomp_test. I can't imagine that is related to this PR but it should be investigated. It seems likely to have been introduced by some other recent E3SM PR.

@mark-petersen
Copy link
Collaborator

@xylar thanks. Yes, underscores are better. What compiler and machine did you see the failure, and debug or optimized? I also saw a QU240 decomp failure but only with intel optimized. I did not see any with the baroclinic channel, testing master today.

@xylar
Copy link
Collaborator Author

xylar commented Oct 6, 2022

@mark-petersen, as I said above, Anvil, Intel, Intel-MPI. I was running optimized.

@xylar
Copy link
Collaborator Author

xylar commented Oct 6, 2022

I can use #354 to find out which E3SM merge introduced this.

@mark-petersen
Copy link
Collaborator

mark-petersen commented Oct 6, 2022

See issue on test failure at E3SM-Project/E3SM#5219

@xylar xylar force-pushed the add_time_daily_restart_test branch from e96b232 to 5e8072e Compare October 20, 2022 12:41
@xylar xylar marked this pull request as draft December 13, 2022 16:22
@xylar xylar added in progress This PR is not ready for review or merging ocean and removed python package DEPRECATED: PRs and Issues involving the python package (master branch) labels Dec 13, 2022
This test case makes sure the timeSeriesStats* analysis member
produces the same results with a full 8-hour run and with two 4-hour
runs with a restart in between.
One version of each restart test does not include analysis restart
(the configuration that E3SM uses) and another does (the default
in standalone MPAS-Ocean).
@xylar xylar force-pushed the add_time_daily_restart_test branch from 5e8072e to 9f45a8d Compare December 28, 2022 09:09
@xylar
Copy link
Collaborator Author

xylar commented Jun 17, 2024

I am going to close this for now, as no progress has been made in more than a year.

@xylar xylar closed this Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E3SM PR required enhancement New feature or request in progress This PR is not ready for review or merging ocean
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants