-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
impl Power for U256 #4900
Closed
Closed
impl Power for U256 #4900
Changes from 7 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
985d03a
impl Power for U256
andrewvious 35b0dfe
tests
andrewvious 7f4d462
tests
andrewvious d2065be
tests
andrewvious c86bbdb
zero test
andrewvious ad75f19
fixed test
andrewvious 5809cf0
added documentation and helper methods
andrewvious 6a733e6
reworking
andrewvious c9e7206
reworking fn
andrewvious 91747c3
rewrite pow fn
andrewvious 96dae95
rework tweak
andrewvious b234606
merge conflict
andrewvious 64dd5cf
add missing bang, remove ref
andrewvious 83b6c18
update 5^28 test
andrewvious 222eed7
separate pow tests
andrewvious b71d298
try shift equals & direct size assignment
andrewvious fd4fe9d
remove u64 assignment
andrewvious File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
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.
How you tested this? I tried testing this on Wolfram and got this: https://www.wolframalpha.com/input?i=5%5E28+%3D%3D+2*18446744073709551615%2B359414837200037395 where
18446744073709551615
= u64::MAX.So shouldn´t this line be?
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.
🤔 each slot in the
U256
type is au64
which at max is 18446744073709551615. The total number 5^28 can be represented by twou64::MAX
and one with 359414837200037395. At least, that's my understanding from looking at the test. Would you mind explaining howU256 { 0, 0, 2, 359414837200037395 }
is equal to 5^28?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.
Also odd that this test passes if that's true
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 got from Wolfram: https://www.wolframalpha.com/input?i=5%5E28+%3D%3D+2*18446744073709551615%2B359414837200037395.
Another option is https://www.wolframalpha.com/input?i=convert+5%5E28+to+base+18446744073709551615%5D
Where
359414837200037395
is the least significant digit; and2
and the second least significant digit.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.
Another good indication is: https://www.wolframalpha.com/input?i=log_5%5B18446744073709551615%5D
So
u64::MAX
is super close to5^27
.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.
Ah, I see now. You're using the maximum value of
u64
as the base. Why is that?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.
u64::MAX
is 10996163476785723490 greater than 5^27 🤔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.
Isn´t this what U256 is doing with four u64? Each field is a u64 digit.
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 suppose I misunderstood since their test cases seem to imply that every next
u64
is catching the overflow of the previous. I'm mostly asking out of curiosity since this area isn't my forte @xunilrj , sorry to have started such a long thread on your PR @andrewvious 😅This could imply a few different problems:
assert_eq
may be passing incorrectly (I think this is most likely, which could make the last problem true)Power
is somehow inaccurateThere 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.
No worries @eureka-cpu, I appreciate both of you @xunilrj I see how it works now, where on overflow it just increments the next u64 over by one. For example, adding 1 to
u64::MAX
would read: