You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
As discussed in 12229, we find some problems with ApproxRoot here:
ApproxRoot behavior is imo unsafe, it needs to be returning an error or a flag at minimum, if it can't converege within max iterations, so the caller doesn't have incorrect expectations.
ApproxRoot will panic "out of bound" for very large sdk.Dec even if it has smaller bit len than maxDecBitLen. I think this is because we're using Newton-Raphson method with the initial guess of 1, which in cases of large input, makes the second guess far far exceeds the actual root value, causing the "out of bound".
There should be different dedicated functions for square root and cube root.
There's inaccuracy due to not enough precision. Tho this will likely be solved by moving to APD which the sdk team is planning/ working on, so we leave this one out.
Proposal
Make 2 new functions for square root and cube root using different logic than ApproxRoot.
Change ApproxRoot logic so that it doesn't panic out of bound for very large valid sdk.Dec. We can fix this by including some condition checks for the guesses in the function.
Return an error or a flag in case ApproxRoot fails to return the correct result.
For Admin Use
Not duplicate issue
Appropriate labels applied
Appropriate contributors tagged
Contributor assigned/self-assigned
The text was updated successfully, but these errors were encountered:
Problem Definition
As discussed in 12229, we find some problems with
ApproxRoot
here:ApproxRoot
behavior is imo unsafe, it needs to be returning an error or a flag at minimum, if it can't converege within max iterations, so the caller doesn't have incorrect expectations.ApproxRoot
will panic "out of bound" for very largesdk.Dec
even if it has smaller bit len thanmaxDecBitLen
. I think this is because we're using Newton-Raphson method with the initial guess of 1, which in cases of large input, makes the second guess far far exceeds the actual root value, causing the "out of bound".Proposal
ApproxRoot
.ApproxRoot
logic so that it doesn't panicout of bound
for very large validsdk.Dec
. We can fix this by including some condition checks for the guesses in the function.ApproxRoot
fails to return the correct result.For Admin Use
The text was updated successfully, but these errors were encountered: