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

Bug fixes for time series conservation task #1014

Merged
merged 3 commits into from
Jul 18, 2024

Conversation

cbegeman
Copy link
Collaborator

Since we have changed the output frequency for the conservation AM to daily E3SM-Project/E3SM#6412, I have noticed that this causes a bug with time averaging. The time averaging config option movingAveragePoints assumes that the output frequency for all the time series tasks is the same. This PR introduces a factor of 30 to movingAveragePoints for the conservation task and fixes an issue with the time string correction for restarts.

@cbegeman cbegeman requested a review from xylar July 17, 2024 20:58
@cbegeman
Copy link
Collaborator Author

@xylar I'm assigning this to you because I'm leaving on vacation. Hopefully the review will be easy.

@cbegeman cbegeman force-pushed the conserv-output-freq-fixes branch from 375d949 to ff6c80d Compare July 17, 2024 21:27
@xylar xylar force-pushed the conserv-output-freq-fixes branch from 3dca96b to 15cfd88 Compare July 18, 2024 14:21
Copy link
Collaborator

@xylar xylar left a comment

Choose a reason for hiding this comment

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

I tested this with the test suite, after switching to a newer QU240wLI simulation that has the conservation check turned on. Results are here:
https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.xasay-davis/analysis_testing/chrysalis/conserv-output-freq-fixes/
Notably, the conservation analysis is there and seems to be averaged over 365 days:
https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.xasay-davis/analysis_testing/chrysalis/conserv-output-freq-fixes/main_py3.11/ocean/index.html#timeseries

I also ran just the conservation analysis (following an example I found from @cbegeman) for high res test 11. I ran both with a 365 day window and a 1-day window to make sure I saw the expected differences. Indeed, I did.

Here's 365:
https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.xylar/analysis_testing/20240626.HRr5-test11.chrysalis_conserv_test/ocean/index.html
Here's 1:
https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.xylar/analysis_testing/20240626.HRr5-test11.chrysalis_conserv_test_window_1/ocean/index.html

@cbegeman
Copy link
Collaborator Author

Thanks @xylar for fixing up syntax and formatting as well as testing. Feel free to merge whenever you'd like.

@xylar
Copy link
Collaborator

xylar commented Jul 18, 2024

Yep, sorry about that. I meant to merge already :-)

@xylar xylar merged commit 22b15ef into MPAS-Dev:develop Jul 18, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants