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

number.float is unable to return 1/max exactly #2361

Closed
9 of 10 tasks
ST-DDT opened this issue Aug 31, 2023 · 6 comments · Fixed by #2474
Closed
9 of 10 tasks

number.float is unable to return 1/max exactly #2361

ST-DDT opened this issue Aug 31, 2023 · 6 comments · Fixed by #2474
Assignees
Labels
c: docs Improvements or additions to documentation m: number Something is referring to the number module p: 1-normal Nothing urgent
Milestone

Comments

@ST-DDT
Copy link
Member

ST-DDT commented Aug 31, 2023

Pre-Checks

Describe the bug

faker.number.float() is unable to return 1.

Minimal reproduction code

it('should be able to return 1', () => {
  // @ts-expect-error: access private member field
  const mersenne = faker._mersenne;
  mersenne.next = () => 0.9999999999999999; // based on the definition of mersenne.next()
  const actual = faker.number.float();
  expect(actual).toBe(1); // 💥 fails
});

Additional Context

Originally part of:

Environment Info

System:
    OS: Windows 10 10.0.22621
    CPU: (16) x64 Intel(R) Core(TM) i7-7820X CPU @ 3.60GHz
    Memory: 47.42 GB / 63.69 GB
  Binaries:
    Node: 18.17.1 - C:\Program Files\nodejs\node.EXE
    npm: 9.7.2 - C:\Program Files\nodejs\npm.CMD
    pnpm: 8.5.1 - C:\Program Files\nodejs\pnpm.CMD
  Browsers:
    Edge: Spartan (44.22621.2134.0), Chromium (116.0.1938.62)
    Internet Explorer: 11.0.22621.1

Which module system do you use?

  • CJS
  • ESM

Used Package Manager

pnpm

@ST-DDT ST-DDT added c: bug Something isn't working help wanted Extra attention is needed p: 1-normal Nothing urgent m: number Something is referring to the number module labels Aug 31, 2023
@dubzzz
Copy link

dubzzz commented Sep 7, 2023

It seems to be the usual behaviour for a random number generator generating values between 0 (included) and 1 (not included). For instance, Math.random does exactly the same.

@ST-DDT
Copy link
Member Author

ST-DDT commented Sep 7, 2023

Then the problem is all our other methods such as faker.number.int that do generate the max value?
What do you think?

@Shinigami92
Copy link
Member

Shinigami92 commented Sep 7, 2023

Yeah, I came to the same conclusion as @dubzzz
It is very unusual that you every need a 1.0, it is always 0.0 inclusive to 1.0 exclusive.
Now as I worked a little bit with #2379 and also had a look into https://www.npmjs.com/package/pure-rand, I'm more and more of the opinion that a 1.0 inclusive is not wanted at all and if our number.float allows an inclusive max, then maybe we need to change that or find a real reason why it is that way.

I didn't head an eye on that until now, because I was always just interested in some fake data and not scientific correct accurate numbers. Just like give me any value between 0 and 1, I don't care if it ever bulls-eye 0.0, 0.5 or 1.0 exactly.

@matthewmayer
Copy link
Contributor

Ints are fine to have inclusive max.

But floats intuitively don't.

@ST-DDT
Copy link
Member Author

ST-DDT commented Sep 7, 2023

What about our faker.number.float() method with precision?

@Shinigami92
Copy link
Member

Team Decision:

Works as expected
We need to enhance our documentation for that

@ST-DDT ST-DDT added c: docs Improvements or additions to documentation and removed c: bug Something isn't working help wanted Extra attention is needed labels Sep 19, 2023
@ST-DDT ST-DDT self-assigned this Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: docs Improvements or additions to documentation m: number Something is referring to the number module p: 1-normal Nothing urgent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants