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

faker.number.int() returns numbers divisible by 2^[1...21] for high maximums #3367

Closed
10 tasks done
michal-kocarek opened this issue Jan 8, 2025 · 4 comments
Closed
10 tasks done
Labels
c: bug Something isn't working m: number Something is referring to the number module

Comments

@michal-kocarek
Copy link

michal-kocarek commented Jan 8, 2025

Pre-Checks

Describe the bug

This investigation started by finding out that faker.number.int() % 4 is always true, regardless of the seed, regardless of how many times I query the value.

We had following code in our tests:

const { faker } = require('@faker-js/faker');

function rand(N) {
   return faker.number.int() % N;
}

// No matter how many times called, which seed is given, this always applies:
rand(4) === 0;

The issue happens only for pretty large numbers (see my investigation below). For example, if I call it with max: 10000, numbers are correctly divisible. I suspect that the issue is somewhere when MAX_SAFE_INT bounds are exceeded within number.int() calculation. There is multiplication of the random number, which (probably) loses the precision.


I know that this isn't how to use faker. I should have provided min and max values to int() method instead, which is what am doing right now. However, this is something that should at least deserve warning in the documentation, so people know that for numbers higher than x, precision (and randomness) is being lost.

Minimal reproduction code

const assert = require('node:assert');
const { faker } = require('@faker-js/faker');

function rand(N) {
  return faker.number.int() % N;
}

for (let i = 0; i < 100; ++i) {
  assert(rand(4) === 0);
}
console.log('All 100 random numbers are divisible by 4');

Additional Context

I was playing with following script to see how big precision is being lost. You can move EXPONENT variable up or down and will visually see that remainders start to become random.

const { faker } = require('@faker-js/faker');

const EXPONENT = 21;

faker.seed();

console.log(`faker.number.int() % (2^ EXPONENT = ${EXPONENT} )`);
for (let i = 1; i <= 100; i++) {
  const number = faker.number.int({ min: 0, max: Number.MAX_SAFE_INTEGER });
  console.log(`${i.toString().padEnd(3)} ${number.toString().padEnd(16)} remainder ${number % Math.pow(2, EXPONENT)}`);
}
2 ... 10 ... 21 22 23 ...
image ... image ... image image image ...

As an outcome, we know that unbound faker.numbner.int() always returns numbers fully divisble by 2, 4, 8, ...2^21.

Environment Info

System:
    OS: macOS 15.2
    CPU: (10) arm64 Apple M1 Pro
    Memory: 106.48 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.18.0 - ~/.nvm/versions/node/v20.18.0/bin/node
    npm: 10.8.2 - ~/.nvm/versions/node/v20.18.0/bin/npm
    pnpm: 9.7.1 - ~/Library/pnpm/pnpm
  Browsers:
    Chrome: 131.0.6778.206
    Safari: 18.2
  npmPackages:
    @faker-js/faker: ~8.4.1 => 8.4.1

Which module system do you use?

  • CJS
  • ESM

Used Package Manager

pnpm

@michal-kocarek michal-kocarek added c: bug Something isn't working s: pending triage Pending Triage labels Jan 8, 2025
@michal-kocarek michal-kocarek changed the title faker.number.int returns predictable numbers for very high maximums faker.number.int returns numbers divisible by 2^[1...21] for high maximums Jan 8, 2025
@michal-kocarek michal-kocarek changed the title faker.number.int returns numbers divisible by 2^[1...21] for high maximums faker.number.int() returns numbers divisible by 2^[1...21] for high maximums Jan 8, 2025
@ST-DDT
Copy link
Member

ST-DDT commented Jan 9, 2025

@ST-DDT ST-DDT added s: awaiting more info Additional information are requested m: number Something is referring to the number module labels Jan 9, 2025
@michal-kocarek
Copy link
Author

michal-kocarek commented Jan 9, 2025

I have checked the number generation in v9, and it works correctly. High numbers returned by faker.number.int() behave much more as expected.

image

I apologize for not running the bug in latest version first.
I think this bug can be closed.

@ST-DDT ST-DDT removed s: pending triage Pending Triage s: awaiting more info Additional information are requested labels Jan 9, 2025
@ST-DDT
Copy link
Member

ST-DDT commented Jan 9, 2025

No problem thanks for your detailed testing again.

If you need the same behavior for v8 you can create a new Faker instance and pass the 53bit randomizer.

faker/src/faker.ts

Lines 155 to 163 in 98ff809

/**
* The Randomizer to use.
* Specify this only if you want to use it to achieve a specific goal,
* such as sharing the same random generator with other instances/tools.
*
* @default generateMersenne32Randomizer()
*/
randomizer?: Randomizer;
});

@michal-kocarek
Copy link
Author

Thank you for your quick reply and advice.

The quickfix for v8 was to use smaller range. We replaced faker.number.int() % 4 with faker.number.int({min: 0, max: 3}).

As conceptual fix, we'll update to v9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: bug Something isn't working m: number Something is referring to the number module
Projects
None yet
Development

No branches or pull requests

2 participants