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

DM-36944: Update lookup table comparison values after changing to float64. #97

Merged
merged 1 commit into from Nov 9, 2022

Conversation

erykoff
Copy link
Contributor

@erykoff erykoff commented Nov 9, 2022

The values are now computed in fgcm using float64 internally, which removes some floating-point rounding errors. The new values are "more correct" and also consistent across scipy versions.

The values are now computed in fgcm using float64 internally, which
removes some floating-point rounding errors.  The new values are
"more correct" and also consistent across scipy versions.
I0STD = [0.08294534, 0.07877351, 0.06464688]
I10STD = [-0.000091981, -0.00061516, -0.00063434]
I0RECON = [0.07322632, 0.0689530429, 0.05600673]
I10RECON = [-5.89806616479, -7.01786443508, 3.62738180611]
Copy link

Choose a reason for hiding this comment

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

I do see that the I10RECON numbers are slightly different from what was there before; I don't know enough to say which values are right.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are computed by explicitly doing the simpson integration with all float64 inputs, and I'm assuming that is "more correct". Also, they're now consistent across scipy versions of the simpson integration.

@erykoff erykoff merged commit 2ffcdf4 into main Nov 9, 2022
@erykoff erykoff deleted the tickets/DM-36944 branch November 9, 2022 19:33
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.

None yet

2 participants