div and mod - wrong results for negative numbers #16

Closed
uhucrew opened this Issue Aug 28, 2012 · 5 comments

Comments

Projects
None yet
3 participants
@uhucrew

uhucrew commented Aug 28, 2012

div and mod operations on negative numbers give always wrong results.

Code:
var bigint= require('bigint');

var big= bigint('-178');
var bigModulo= bigint('10');
var resDiv= big.div(bigModulo);
var resMod= big.mod(bigModulo);
console.log(big.toString() + ' div ' + bigModulo.toString() + ' = ' + resDiv.toString());
console.log(big.toString() + ' mod ' + bigModulo.toString() + ' = ' + resMod.toString());

Output:
-178 div 10 = -18
-178 mod 10 = 2

@freeeve

This comment has been minimized.

Show comment Hide comment
@freeeve

freeeve Oct 7, 2012

Contributor

I'm pretty sure those answers are correct. What did you expect them to be?

Contributor

freeeve commented Oct 7, 2012

I'm pretty sure those answers are correct. What did you expect them to be?

@uhucrew

This comment has been minimized.

Show comment Hide comment
@uhucrew

uhucrew Oct 8, 2012

Because no exact mathematical definition for div/mod exists, this result is one of the possible correct results.
But the JavaScript implementation for numbers is different.

I expect same results for bigint and Number type. Numbers give this result:

~ $ node

x= -178
-178
x % 10
-8

uhucrew commented Oct 8, 2012

Because no exact mathematical definition for div/mod exists, this result is one of the possible correct results.
But the JavaScript implementation for numbers is different.

I expect same results for bigint and Number type. Numbers give this result:

~ $ node

x= -178
-178
x % 10
-8

@freeeve

This comment has been minimized.

Show comment Hide comment
@freeeve

freeeve Oct 8, 2012

Contributor

Well, even still, this is more of an issue for the underlying GMP library. They've decided to handle it one way. As a wrapper library, node-bigint shouldn't care about that sort of thing. If you really want it to be negative, it's a simple if/else in your app code. Just my opinion, of course--I'm not the author nor a maintainer.

Contributor

freeeve commented Oct 8, 2012

Well, even still, this is more of an issue for the underlying GMP library. They've decided to handle it one way. As a wrapper library, node-bigint shouldn't care about that sort of thing. If you really want it to be negative, it's a simple if/else in your app code. Just my opinion, of course--I'm not the author nor a maintainer.

@uhucrew

This comment has been minimized.

Show comment Hide comment
@uhucrew

uhucrew Oct 9, 2012

I didn't realized that node-bigint is just a wrapper for GMP. So this issue is not related to node-bigint nor it is really an error. It is just confusing that numbers and bigint behave different. Thank you for clarifying.

uhucrew commented Oct 9, 2012

I didn't realized that node-bigint is just a wrapper for GMP. So this issue is not related to node-bigint nor it is really an error. It is just confusing that numbers and bigint behave different. Thank you for clarifying.

@uhucrew uhucrew closed this Oct 9, 2012

@Yaffle

This comment has been minimized.

Show comment Hide comment
@Yaffle

Yaffle Oct 12, 2014

I cannot see "mpz_div" at https://gmplib.org/manual/Integer-Division.html#Integer-Division
but i can see "mpz_tdiv" (truncated divsioin - -1/3 -> 0), "mpz_cdiv" (ceiling division: 1/3->1), and "mpz_fdiv" (floor division -1/3 -> -1)

and the document at https://gitorious.org/linux-nios2/uclinux-dist/source/b59e4526a860f7dca8cf5d46e0e674e026b8622a:lib/libgmp/gmp.info-1 says:

Integer division functions round the result differently.  The
     obsolete functions (`mpz_div', `mpz_divmod', `mpz_mdiv',
     `mpz_mdivmod', etc) now all use floor rounding (i.e., they round
     the quotient towards -infinity).  There are a lot of functions for
     integer division, giving the user better control over the rounding.

Yaffle commented Oct 12, 2014

I cannot see "mpz_div" at https://gmplib.org/manual/Integer-Division.html#Integer-Division
but i can see "mpz_tdiv" (truncated divsioin - -1/3 -> 0), "mpz_cdiv" (ceiling division: 1/3->1), and "mpz_fdiv" (floor division -1/3 -> -1)

and the document at https://gitorious.org/linux-nios2/uclinux-dist/source/b59e4526a860f7dca8cf5d46e0e674e026b8622a:lib/libgmp/gmp.info-1 says:

Integer division functions round the result differently.  The
     obsolete functions (`mpz_div', `mpz_divmod', `mpz_mdiv',
     `mpz_mdivmod', etc) now all use floor rounding (i.e., they round
     the quotient towards -infinity).  There are a lot of functions for
     integer division, giving the user better control over the rounding.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment