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

Use f64 instead of u64 in PascalRowIter to avoid integer overflow #9

Merged
merged 1 commit into from
May 12, 2024

Conversation

lars-frogner
Copy link
Contributor

Something like this works up to n = 131 when F is f32 and up to n = 1020 when F is f64. Above these limits pascal exceeds the highest representable value and becomes infinity.

Copy link
Owner

@ickk ickk left a comment

Choose a reason for hiding this comment

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

Thanks again Lars!

Regarding this approach, I'm not sure exactly what effect the inexactness of floats has on the quality of the output, but given we use this only for generating initial guesses it's probably a good compromise

Eventually I'll read through Dario Bini's paper (DOI:10.1007/BF02207694) #2 which I think replaces the procedure for generating initial guesses, and if we're lucky it may serve to further relax this constraint, but there's no rush. At the moment we expose the internal::aberth_raw function, so someone sufficiently motivated could supply their own initial guesses to work around the limitation if they needed

@ickk ickk merged commit cb50b6b into ickk:dev May 12, 2024
1 check passed
@lars-frogner
Copy link
Contributor Author

Just to be sure, I compared the outputs of the f64 based implementation of the iterator with that of the u64 based implementation for all n < 64. They agree to within a relative error of 1e-15, so the inexactness shouldn't pose an issue as far as I can tell.

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.

2 participants