Skip to content

Use consistent precision in multi-model statistics calculation and update reference data for tests #941

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.

Sign up for GitHub

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

Jump to bottom
Merged
nielsdrost merged 2 commits into from
Jan 13, 2021
Merged

Use consistent precision in multi-model statistics calculation and update reference data for tests #941

nielsdrost merged 2 commits into from
Jan 13, 2021

Conversation

@Peter9192
Copy link
Contributor

@Peter9192 Peter9192 commented Jan 12, 2021 •
edited
Loading

This PR forces the precision in the multi-model statistics calculation for span=full to float32. Also the reference data for the sample-data tests are updated. This is needed to verify that no (other) unwarranted changes to the multi-model statistics functionality are introduced in #685

closes #940

Checklist

If you make backwards incompatible changes to the recipe format:

  • N/A Update ESMValTool and link the pull request(s) in the description

To help with the number pull requests:

@Peter9192 Peter9192 requested a review from stefsmeets January 12, 2021 16:50
@Peter9192 Peter9192 mentioned this pull request Jan 12, 2021
Merged
6 tasks
stefsmeets
stefsmeets approved these changes Jan 13, 2021
View reviewed changes
Copy link
Contributor

@stefsmeets stefsmeets left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good to me, because we already change the dtype here anyway:

ESMValCore/esmvalcore/preprocessor/_multimodel.py

Line 439 in 4d52e4a

statistic_cube.data = np.ma.array(statistic_cube.data,

Is that what you mean? Is there a reason we do not keep the type of the incoming data?

@Peter9192
Copy link
Contributor Author

Peter9192 commented Jan 13, 2021

Is there a reason we do not keep the type of the incoming data?

New numpy arrays default to float64, so I think it's just gone unnoticed. There is an argument to be made for calculating on higher precision, but it doesn't seem to be the reason here, because for span='overlap' it just calculates on float32.

@Peter9192
Copy link
Contributor Author

Peter9192 commented Jan 13, 2021

@nielsdrost could you merge this please?

@Peter9192
Copy link
Contributor Author

Peter9192 commented Jan 13, 2021

I created an issue #943 to discuss whether it would be desirable to intentionally enforce high precision calculations.

@Peter9192 Peter9192 changed the title Correct precision of multi-model statistics calculation and update reference data for tests Use consistent precision in multi-model statistics calculation and update reference data for tests Jan 13, 2021
@nielsdrost nielsdrost merged commit 7770d6d into master Jan 13, 2021
@nielsdrost nielsdrost deleted the mmstats_change_precision branch January 13, 2021 10:37
@jvegreg jvegreg added the preprocessor Related to the preprocessor label Feb 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@stefsmeets stefsmeets stefsmeets approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

preprocessor Related to the preprocessor

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Multimodel stats computes with artificially high precision.

5 participants

@Peter9192 @stefsmeets @nielsdrost @jvegreg