multipleOf test is invalid

[karenetheridge]

I just added support in my implementation for very very very large numbers, which got the test in optional/float-overflow.json passing, but tickled the final test case in multipleOf.json (which divides 1e308 by 0.123456789) -- I don't see how I can pass both these tests, as either I overflow (which should return valid:false), or I don't overflow, and get valid: true...

because, actually, 1e308 divided by 0.123456789 is an integer:
810000007371000067076100610392515554571900000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

see discussion in #438
cc @Zac-HD

[karenetheridge]

how about: 1e308 / 7e307, which is pretty easily calculated by eye to equal = 1.428571...

[ChALkeR]

because, actually, 1e308 divided by 0.123456789 is an integer:
810000007371000067076100610392515554571900000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

No, it's not (at least not that):

> 810000007371000067076100610392515554571900000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000n * 123456789n
100000000000000000000000000000000000000001043629100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000n

See the ...10436291... part.

This is also the bigint arithmetic, so it's exact.

A slimmed down example:

> 8100000073710000670761006103925155545719n*123456789n
1000000000000000000000000000000000000000010436291n

If the calc in the issue was correct, this should have resolved to 1000000000000000000000000000000000000000000000000n.

[karenetheridge]

Sorry, I don't know how to read that. what is n?

[ChALkeR]

n is just a notation for using big int (exact long integer arithmetic in js).
And a/b = c could be disproved by checking that b*c does not equal to a, which is basically what I did with multiplication there: answers didn't match.

But here is a simpler explanation that doesn't even deal with long numbers:

1. 1e308 / 0.123456789 = 1e308 / (123456789 / 1e9) = 1e308 * 1e9 / 123456789 = 1e317 / 123456789
2. 123456789 is dividable by 3 (123456789 = 3*3*3607*3803), and 1e317 is obviously not.
3. As 1e317 is not dividable by 3, hence, 1e317 is not dividable by 3*3*3607*3803 = 123456789, hence, 1e308 is not dividable by 0.123456789.

[ChALkeR]

I believe that whatever check you used just lost the precision somewhere, and the test is correct while the implementation isn't.

[ChALkeR]

Here is a divison check in js (I did 1e317 % 123456789 to fit into integers, but that should have been equal to zero if and only if 1e308 % 0.123456789 is equal to zero):

> BigInt('1' + '0'.repeat(317)) % 123456789n
88581385n

That's the remainder we got.

[ChALkeR]

810000007371000067076100610392515554571900000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

This is what it actually looks like:

> BigInt('1' + '0'.repeat(317)) / 123456789n
810000007371000067076100610392515554571891546604213074098338974294884666083450461359399198370532705171847617063813315280701169054380638394863809393260665478672055855915708288832945428379803398256210924131519409596826627331122308713213009290238384541169299324640623854229677073490061368759558455711981946979035n

Plus a remainder.

The lib you are using lost precision somewhere after 131 bits perhaps? (If I got my numbers right).

[karenetheridge]

aha, I fiddled with my bignum library some more, and got a better result:
1e308 / 0.123456789 = 810000007371000067076100610392515554571891546604213074098338974294884666083450461359399198370532705171847617063813315280701169054380638394863809393260665478672055855915708288832945428379803398256210924131519409596826627331122308713213009290238384541169299324640623854229677073490061368759558455711981946979035 with a remainder of 0.088581385.

That's still not the same answer as yours though.

[karenetheridge]

Given that this test is correct, we should have another multipleOf test that would also normally overflow, but produces a valid result (the quotient is an integer).

edit: I no longer agree with this. A test that expects valid: true would be forcing bignum support, which I don't think we want to do. The existing test that expects valid: false is correct under both scenarios: 1. the implementation supports bignums, and gets the correct result, or 2. the implementation detects an overflow situation and returns false.

[ChALkeR]

I just checked my impl, it passes on the tests that are present here: 1e308 % 0.5 == 0, 1e308 % 0.123456789 != 0, but fails on e.g. 1e308 % 0.4 (where we overflow with a divisor that is not a power of two times an integer).

          "schema": {"type": "integer", "multipleOf": 0.4},
          "tests": [
              {
                  "data": 1e308,
                  "valid": true
              }
          ]

We should also include that, I think =).

Upd: fixed my impl there.

[ChALkeR]

Also, this seems to fail on my impl:

          "schema": {"type": "integer", "multipleOf": 3},
          "tests": [
              {
                  "data": 1e306,
                  "valid": false
              }
          ]

This happens because 1e306 == 6413338752028713 * 2**964 in IEEE 754 double, and 6413338752028713 is dividable by 3.

I'll think what could be done with that without having to implement arbitrary precision floats which are very slow.

[karenetheridge]

The lib you are using lost precision somewhere after 131 bits perhaps? (If I got my numbers right).

Yes, there was an internal setting for division accuracy which was defaulting to 40 digits.

Now I'm pondering what I should set it to for the multipleOf operation specifically in order to guarantee I get enough significant digits to determine if the quotient is an integer. It's not as simple as int(log($data)/log(10)) but it's something similar to that. I need to take into consideration the number of digits (before the decimal point) in the dividend, as well as all the significant digits (before and after the decimal point) in the divisor.

edit: actually none of that is necessary :) -- I found a way to get the remainder from the division operation without having to increase the accuracy. All that we care about in multipleOf is if the remainder is 0 or not.

[karenetheridge]

My original assertion in this issue is false, and I am satisfied that there were no unsolveable issues raised here, so I'm closing. I'll be sending a PR to refine the multipleOf tests to add more combinations and move the overflow test to optional.