-
Notifications
You must be signed in to change notification settings - Fork 195
Use zero() for type-stable calculation of _momentN()
#969
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
Conversation
src/moments.jl
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, s does not have the correct/desired type here as this initialization purely depends on m and neglects v.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was making the assumption that m and v would have compatible/identical types, since m is calculated from v and the function is only called internally. Would an implementation involving v be something like zero(v[begin] * v[begin])?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO we shouldn't make that assumption. For instance, in var(...; mean=...) users can easily provide a mean that is of different type.
Would an implementation involving v be something like zero(v[begin] * v[begin])
No, that would error if the array is empty and would neglect the type of m.
Uh oh!
There was an error while loading. Please reload this page.
Solves #968