Skip to content
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

regression (jaq 1.6 gave correct answer) #240

Open
pkoppstein opened this issue Nov 30, 2024 · 4 comments
Open

regression (jaq 1.6 gave correct answer) #240

pkoppstein opened this issue Nov 30, 2024 · 4 comments

Comments

@pkoppstein
Copy link

Presumably there's an explanation for why jaq 1.6 gave the correct
answer for a particular problem at
https://rosettacode.org/wiki/Sum_multiples_of_3_and_5#jq (reproduced
below), whereas jaq 2.0.0 gives a very different (incorrect) answer,
but is it a sufficiently good one?

$ ./jaq --version
jaq 1.6.0
2.333333333333333e41

$ jaq --version
jaq 2.0.0
-4.611686018427388e18

##########################

def sum_multiples(d):
 ((./d) | floor) |  (d * . * (.+1))/2 ;

# Sum of multiples of a or b that are less than . (the input)
def task(a;b):
 . - 1
 | sum_multiples(a) + sum_multiples(b) - sum_multiples(a*b);

(10e20 | task(3;5))
@wader
Copy link
Contributor

wader commented Dec 1, 2024

Tried to minimize it down a bit:

$ jaq -n '1e20/2 | floor, trunc'
9223372036854775807
5e19
$ jq -n '1e20/2 | floor, trunc'
5e+19
5e+19
$ gojq -n '1e20/2 | floor, trunc'
50000000000000000000
50000000000000000000

with 1e19/2 they all agree

@01mf02
Copy link
Owner

01mf02 commented Dec 2, 2024

I currently do not know why jaq 1.6 seems to output the correct answer, but when you change floor to trunc in your program, then jaq 2.0 also outputs the result 2.333333333333333e41. Why? round, floor, and ceil convert their inputs to integers, but the size of the numbers here is too large for integers --- so we get an overflow, which is why jaq 2.0 outputs a negative number with floor.

As a rule of thumb: When you treat numbers that might not fit into machine-size integers, then use trunc to round them. Otherwise, use round, floor, and ceil.

@null-dev
Copy link
Contributor

null-dev commented Dec 2, 2024

I currently do not know why jaq 1.6 seems to output the correct answer.

I wonder if this was also due to #219 lol?

@01mf02
Copy link
Owner

01mf02 commented Dec 3, 2024

@null-dev, your hypothesis looks quite plausible!

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

No branches or pull requests

4 participants