Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Improved integer square root. #4403
Improved integer square root. #4403
Changes from 15 commits
171e5c2
beec057
a48fda8
b6c29b1
0d2bbb3
1c8e8ab
8e2ab78
77db52d
8b3f15f
dc098e8
bd9d969
dfe2fbc
a53d378
6c74012
fe8abc3
382ab6f
c60542b
ddea292
360f65d
4288c74
256189a
6d00bbd
6c8be26
67d0328
85a56d6
e748bfc
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
This is very simmilar to the code we have in log2. In fact, its actually
1 << (log2(a) >> 1)
. In log2, we changed the logic to branchless, and that saved some gas. Surprisingly the same approach doesn't appear to save much here.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.
Again, I don't think that is clear.
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.
My understanding is that we know
result <= sqrt(a) < 2 * result
... so right now we have this distance:However, if we take the middle of that interval, then the distance becomes
But that still only gives us
result/2 = 2**(e-1)
... where does2**(e-2)
come from?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.
Right. But consider we're looking for
e
such that2**e + c == a
and the worst case for c is2**e - 1
. If we're usinge
, we would be above the square root. So you should note this asresult/2 = 2**(e-2)
.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.
We have e such that "2 ** e <= sqrt(a) < 2 ** (e+1)"
Looks like there is a confusion between a and sqrt(a) here.
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.
Actually, the confusion is in the definition of e. Comments in the code and PDF define it differently
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.
The comments should be readable alone, without the PDF document.
Here,
e+1
reads likee
plus the number one, which is not how it should be interpreted.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 think the intention is to express
e_{+1}
. Would you be ok with that syntax?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.
Is that correct?
Where does the initial 4.5 come from ? Its not documented anywhere.
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.
Yes it's correct. Feel free to update it
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.
Added a reference, but comes from using the error formula from Walter Rudin. Principles of Mathematical Analysis. 3rd ed. McGraw-Hill New York, 1976. Exercise 3.16 (b)
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.
Rather than adding a reference (to something that I couldn't easily find ... I mean I found a 352 pages long scanned document, with 3.16 being an unrelated definition ... but that doesn't go in the right direction) I'd rather add a proof.
AFAIK, its faster to reasd the proof than find the reference :P
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.
Note 3.16 is not the exercise but a section. I think you're looking for this:
And the solution:
