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

Mask ocean time series stats variables #5325

Merged
merged 12 commits into from
Dec 5, 2022

Conversation

cbegeman
Copy link
Contributor

@cbegeman cbegeman commented Nov 22, 2022

This PR applies fill value masking to non-restart time series stats streams in the ocean component. It also specifies the masking array for a subset of variables in the Registry.

It builds on the PRs that add the masking option to the framework, #5154 and #5251. Discussion can be found at E3SM-Ocean-Discussion#32.

Fixes #5253

[BFB]

Copy link
Contributor

@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 ran the CMPASO-JRA1p4_TL319_EC30to60E2r2 test case for a month and took a look at masking for a random sampling of different time series stats variables for which masking has been turned on in this PR. Everything looked good to me. Results are at:

/lcrc/group/e3sm/ac.xasay-davis/scratch/chrys/20221129.CMPASO-JRA1p4_TL319_EC30to60E2r2.fix-time-series-stats.chrysalis/run

@xylar
Copy link
Contributor

xylar commented Nov 29, 2022

@darincomeau, can you take a look at this when you get a chance? It would be great to have another test or at least another pair of eyes on the test results I just produced before this goes in.

@darincomeau
Copy link
Member

I ran a one month G case here: /lcrc/group/acme/ac.dcomeau/scratch/chrys/20221130.GMPAS-JRA1p4.TL319_ECwISC30to60E2r1.chrysalis.intel/run

Looking through timeSeriesStatsMonthly I'm seeing the new attributes in various variables that were not previously present, so looks good to me.

@cbegeman
Copy link
Contributor Author

@darincomeau Thanks so much for reviewing!

@rljacob rljacob added this to the v2.1rc milestone Dec 1, 2022
jonbob added a commit that referenced this pull request Dec 1, 2022
…5325)

Mask ocean time series stats variables

This PR applies fill value masking to non-restart time series stats
streams in the ocean component. It also specifies the masking array for
a subset of variables in the Registry.

Fixes #5253

[BFB]
@jonbob
Copy link
Contributor

jonbob commented Dec 1, 2022

passes:

  • SMS_D_Ld3.T62_oQU120.CMPASO-IAF.chrysalis_intel
  • ERS.ne11_oQU240.WCYCL1850NS.chrysalis_intel
  • SMS_D_Ld1.ne30pg2_EC30to60E2r2.WCYCL1850.chrysalis_intel.allactive-wcprod

merged to next

@jonbob jonbob merged commit 627683e into E3SM-Project:master Dec 5, 2022
@jonbob
Copy link
Contributor

jonbob commented Dec 5, 2022

merged to master

@xylar
Copy link
Contributor

xylar commented Dec 5, 2022

Woohoo! Thanks @jonbob and @cbegeman!

@cbegeman
Copy link
Contributor Author

cbegeman commented Dec 5, 2022

@jonbob Thanks for your testing! I'm happy that we're a little closer to CF compliance

@jonbob
Copy link
Contributor

jonbob commented Dec 5, 2022

@cbegeman - you make it very easy

xylar added a commit to xylar/compass that referenced this pull request Dec 15, 2022
This merge updates the E3SM-Project submodule from [569ed6b730](https://github.com/E3SM-Project/E3SM/tree/569ed6b730) to [0273cfad9d](https://github.com/E3SM-Project/E3SM/tree/0273cfad9d).

This update includes the following MPAS-Ocean and MPAS-Frameworks PRs (check mark indicates bit-for-bit with previous PR in the list):
- [ ]  (ocn) E3SM-Project/E3SM#5306
- [ ]  (fwk) E3SM-Project/E3SM#5303
- [ ]  (ocn) E3SM-Project/E3SM#5325
- [ ]  (fwk) E3SM-Project/E3SM#5337
- [ ]  (fwk) E3SM-Project/E3SM#5123
- [ ]  (fwk) E3SM-Project/E3SM#5281
- [ ]  (ocn) E3SM-Project/E3SM#5356
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BFB PR leaves answers BFB bug fix PR mpas-ocean
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fill values are not appearing in fields in timeSeriesStats
5 participants