Skip to content

fix possible overflows in Moments #43

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

brucala
Copy link
Contributor

@brucala brucala commented May 5, 2025

Problem:

If the input type of the observations have maxtype smaller than that of a float (for instance Int), then y2 = y * y can easily overflow resulting in the wrong moments.

Example

m = fit!(Moments(), [4_000_000_000, 4_000_000_100])
Moments: n=2 | value=[4.0e9, -2.44674e18, 1.06766e18, 6.84018e18]
julia> var(m)
-3.6893488147419095e19

julia> std(m)
ERROR: DomainError with -3.6893488147419095e19:
sqrt was called with a negative real argument but will only return a complex result if called with a complex argument. Try sqrt(Complex(x)).

Solution

Simply convert y to a Float64 in the computation of y2. Float64 is actually the type of o.m so the conversion was happening in any case at a later stage. By moving it to an earlier stage possible overflows are avoided.

Caveat

Note that this does not fix possible precision errors inherent to storing the moments as Float64. For instance, the example above after the proposed fix returns var(m) = 4096.0 instead of 5000.
Also, attempting to fit float inputs with scales sqrt(maxfloat(Float64)) will still result in overflows.

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.

1 participant