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

Fix precision for writing field attributes #5251

Merged
merged 1 commit into from
Nov 2, 2022

Conversation

xylar
Copy link
Contributor

@xylar xylar commented Oct 26, 2022

It is necessary that the precision of floating-point attributes match the precision of the field they are associated with. In particular, when the _FillValue has a different precision from the field it is associated with, PIO complains and will not add the attribute.

Fixes #5235
[BFB]

It is necessary that the precision of floating-point attributes
match the precision of the field they are associated with.  In
particular, when the `_FillValue` has a different precision from
the field it is associated with, PIO complains and will not add
the attribute.
@xylar xylar added bug fix PR BFB PR leaves answers BFB mpas-framework labels Oct 26, 2022
@xylar xylar requested a review from cbegeman October 26, 2022 16:23
@xylar
Copy link
Contributor Author

xylar commented Oct 26, 2022

Testing a CMPASO-TL319_EC30to60E2r2 run on Cori. I'll take this out of draft mode if I find out that that run is successful.

@xylar xylar marked this pull request as ready for review October 26, 2022 19:19
@xylar
Copy link
Contributor Author

xylar commented Oct 26, 2022

Testing

The fix does seem to have worked as expected. See results in:

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

a 1-month MPAS-O simulation in which timeSeriesStatsMonthly shows the expected single-precision fill values:

$ ncdump -h 20221026.CMPASO-JRA1p4_TL319_EC30to60E2r2.fix-time-series-stats.chrysalis.mpaso.hist.am.timeSeriesStatsMonthly.0001-01-01.nc | grep "_FillValue"
		timeMonthly_avg_layerThickness:_FillValue = 9.96921e+36f ;
		timeMonthly_avg_normalVelocity:_FillValue = 9.96921e+36f ;
		timeMonthly_avg_activeTracers_temperature:_FillValue = 9.96921e+36f ;
		timeMonthly_avg_activeTracers_salinity:_FillValue = 9.96921e+36f ;

However, I am not seeing the expected fill values in the fields themselves, e.g.:

 ncdump -v timeMonthly_avg_activeTracers_temperature 20221026.CMPASO-JRA1p4_TL319_EC30to60E2r2.fix-time-series-stats.chrysalis.mpaso.hist.am.timeSeriesStatsMonthly.0001-01-01.nc | tail
  27.55785, 27.53584, 27.52425, 27.51439, 27.50551, 27.49708, 27.38161, 
    26.46217, 24.78305, 23.25717, 21.93985, 20.68438, 19.45276, 18.26175, 
    17.16108, 16.16831, 15.26815, 14.42972, 13.62875, 12.85962, 12.13178, 
    11.46993, 10.88002, 10.35534, 9.884884, 9.467792, 9.11305, 8.818574, 
    8.540646, 8.274009, 8.000038, 7.712546, 7.409822, 7.091074, 6.753331, 
    6.39432, 6.000737, 5.534378, 5.060202, 4.568097, 4.028841, 3.540712, 
    3.114681, 2.73235, 2.406448, 2.104124, 1.880408, 1.692876, 1.552158, 
    1.462184, 1.408809, 1.356857, 1.314016, -1e+34, -1e+34, -1e+34, -1e+34, 
    -1e+34, -1e+34, -1e+34 ;

This suggests that work is still needed in timeSeriesStats to make sure that fill masks from the original fields are also used for the time-averaged fields. Since this is a different bug, I won't try to address it here.

@xylar
Copy link
Contributor Author

xylar commented Oct 26, 2022

@cbegeman, could you take a look at this when you can? If you want to run a test, that's fine. If you prefer to look at my test results, that's okay, too.

Copy link
Contributor

@cbegeman cbegeman left a comment

Choose a reason for hiding this comment

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

@xylar This looks great. Thanks for tackling this. I'll get to the related issue #5253 soon.

jonbob added a commit that referenced this pull request Nov 1, 2022
Fix precision for writing field attributes

It is necessary that the precision of floating-point attributes match
the precision of the field they are associated with. In particular, when
the _FillValue has a different precision from the field it is associated
with, PIO complains and will not add the attribute.

Fixes #5235
[BFB]
@jonbob
Copy link
Contributor

jonbob commented Nov 1, 2022

passes sanity testing, merged to next

@jonbob jonbob merged commit c36af2f into E3SM-Project:master Nov 2, 2022
@jonbob
Copy link
Contributor

jonbob commented Nov 2, 2022

merged to master

@xylar xylar deleted the mpas-framework/bugfix-fillval branch November 2, 2022 17:27
@xylar
Copy link
Contributor Author

xylar commented Nov 2, 2022

Thanks @jonbob and @cbegeman!

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.

FillValue mismatch when writing timeSeriesStats in mpas-o
3 participants