Skip to content

Conversation

LiarPrincess
Copy link
Contributor

First batch of tests from Violet - Python VM written in Swift (connected to #98 Using tests from “Violet - Python VM written in Swift”).

Test failures

Binary operations

Let's use the following test as an example (I chose this one at random, there are other tests that are failing, maybe for different reasons):

self.xorTest(lhs: "-1", rhs: "18446744073709551615", expecting: "-18446744073709551616")

attaswift/BigInt says it equals to 0.

Other engines

  • WolframAlpha - BitXor[-1, 18446744073709551615] -> -18446744073709551616 link
  • Python 3.7.4
    Python 3.7.4 (default, Jul 20 2021, 13:20:24)
    [Clang 12.0.0 (clang-1200.0.32.2)] on darwin
    Type "help", "copyright", "credits" or "license" for more information.
    >>> -1 ^ 18446744073709551615
    -18446744073709551616
  • Node - we generated our test with Node, soooo…

Why?

  • -1 in two complement is represented as all 1 with the desired width.
  • 18446744073709551615 is equal to 2^64 − 1 which is 1111111111111111111111111111111111111111111111111111111111111111 (basically 1 repeated 63 times)

So, obviously if both -1 and 18446744073709551615 are all 1 then xor will be 0, just like attaswift/BigInt returns.

Well… not exactly.

18446744073709551615 is positive, so its binary representation has to have 0 prefix in two complement (otherwise it would mean negative number).

With this:

lhs:    11111111111111111111111111111111111111111111111111111111111111111
rhs:    01111111111111111111111111111111111111111111111111111111111111111
result: 10000000000000000000000000000000000000000000000000000000000000000

Shift right

Let's use the following test as an example (I chose this one at random, there are other tests that are failing, maybe for different reasons):

self.shiftRightTest(value: "-1932735284", count: 5, expecting: "-60397978")

This is an interesting case (look at the last number: 7 vs 8):

Engine Result
attaswift/BigInt -60397977
Wolfram Alpha -60397977
Node v17.5.0 -60397978
Python 3.7.4 -60397978
Swift 5.3.2* -60397978
Violet -60397978

(*) This is in Int range, so you can just -1932735284 >> 5 to test it.

Anyway… Wolfram and attaswift give -60397977, but Node, Python, Swift and Violet give -60397978.

Long story short, both answers are correct, it depends on how you round.

Doing this by hand:
1000 1100 1100 1100 1100 1100 1100 1100 >> 5 = 100 0110 0110 0110 0110 0110 0110 rem 0 1100

With sign extension to Int32: 1111_1100_0110_0110_0110_0110_0110_0110, which is (according to Swift repl):

 13> Int32(bitPattern: 0b1111_1100_0110_0110_0110_0110_0110_0110)
$R11: Int32 = -60397978

I think that attaswift uses sign + magnitude representation. If it was 2 complement then everything would be trivial, but it is not, so sometimes you need an adjustment: Swift uses what would be 'GMP_DIV_FLOOR' mode in 'GMP'. Which means that if we are negative and any of the removed bits is '1' then we have to round down.

Wolfram does not round down (which is “more” mathematically correct!).

@LiarPrincess
Copy link
Contributor Author

Anyway… I will work on adding more tests later…

Btw. Take those explanations with a grain of salt. Obviously I wrote those tests (well Node generated them… but it is still my work), so subconsciously I will try to “make them correct” (maybe the better term would be “justify their shortcomings”).

Copy link

@gatleas17 gatleas17 left a comment

Choose a reason for hiding this comment

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

pla

@tothambrus11
Copy link
Contributor

Hi @LiarPrincess , are you still planning to work on this PR? Is there any help needed? I could set up nodejs in the devcontainer and CI if needed. More tests are always better, and it looks like you have already made great progress here :)

@LiarPrincess
Copy link
Contributor Author

I could set up nodejs in the devcontainer and CI if needed.

Those tests are 100% Swift. They were generated by Node because JavaScript also has a BigInt type. Node is not needed to run them. To put is simply: the main thing that this PR adds is 1 big file (generated via Node) with a lot of test cases.

Code in Scripts/bigint_generate_node_tests (part of this PR) is used to re-generate the Swift test files. There is no need to run this again.

Is there any help needed?

The test cases uncovered 2 potential problems:

  • some binary operators (like xor) may not work correctly
  • shifting right (>>) uses different rounding mode than Swift

The core team has to decide what to do with this. If we merge the tests we will have failures. Note that changing the rounding mode for >> is a breaking change!

looks like you have already made great progress here

This PR was a part of a series of PRs that add tests from Violet - Python VM written in Swift. If you go to the Tests/BigIntTests you will see a few directories with "Violet" prefix. I am responsible for them (code ownership) if there are any problems (failures etc.) with them feel free to assign me.

@tothambrus11
Copy link
Contributor

Thanks for the explanation :)

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.

3 participants