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

Generating a float with precision of 0.00001 gives larger precision number #2567

Closed
8 of 10 tasks
clocke3 opened this issue Dec 5, 2023 · 14 comments · Fixed by #2581
Closed
8 of 10 tasks

Generating a float with precision of 0.00001 gives larger precision number #2567

clocke3 opened this issue Dec 5, 2023 · 14 comments · Fixed by #2581
Labels
c: bug Something isn't working has workaround Workaround provided or linked m: number Something is referring to the number module p: 1-normal Nothing urgent
Milestone

Comments

@clocke3
Copy link

clocke3 commented Dec 5, 2023

Pre-Checks

Describe the bug

When trying to generate a float using faker.number.float({ precision: 0.00001 }), the generated float generates a number with more than 5 decimal place values after the decimal point. Generated float values where precision is set to 0.1 to 0.0001, and 0.000001 and greater are consistently working.

Minimal reproduction code

https://runkit.com/embed/g86r0jyau1rk

Additional Context

fake

Using faker-js to write playwright tests; example shows a generated value logged out where it was set to faker.number.float({ max: 99999.99999, precision: 0.00001 }).

Environment Info

System:
    OS: Linux 5.10 Ubuntu 20.04.6 LTS (Focal Fossa)
    CPU: (20) x64 12th Gen Intel(R) Core(TM) i7-1280P
    Memory: 6.67 GB / 15.46 GB
    Container: Yes
    Shell: 5.0.17 - /bin/bash
  Binaries:
    Node: 20.9.0 - /usr/local/bin/node
    Yarn: 3.6.3 - /usr/local/bin/yarn
    npm: 10.2.3 - /usr/local/bin/npm
  npmPackages:
    @faker-js/faker: ^8.0.2 => 8.0.2

Which module system do you use?

  • CJS
  • ESM

Used Package Manager

npm

@clocke3 clocke3 added c: bug Something isn't working s: pending triage Pending Triage labels Dec 5, 2023
@xDivisionByZerox
Copy link
Member

Hi @clocke3 , thank you for your bug.
I'm currently not at home, so I can't verify this (the minimal reproducible example link doesn't work on my phone). I'll give more feedback in a bit.

From your description (and screenshot!) this seems to be legit.
You selected that you are willing to create a PR to fix this. If that's the case, you are welcome to do so.

@ST-DDT
Copy link
Member

ST-DDT commented Dec 5, 2023

The example doesn't work for me either.

Error: Cannot find module '@faker-js/faker@8.3.1'

I assume the bug is related to a float precision issue.

@xDivisionByZerox xDivisionByZerox added p: 1-normal Nothing urgent m: number Something is referring to the number module and removed s: pending triage Pending Triage labels Dec 5, 2023
@xDivisionByZerox xDivisionByZerox added this to the vAnytime milestone Dec 5, 2023
@clocke3
Copy link
Author

clocke3 commented Dec 5, 2023

@xDivisionByZerox I apologize I am new to this issue making and checked the section on "creating a PR for issue" on mistake.

@clocke3
Copy link
Author

clocke3 commented Dec 5, 2023

@ST-DDT Yes it is. Looks like it existed in 8.0.2, 8.2.0, and I did test it out using 8.3.1 locally. I will try to see if I can produce reproduction code using 8.3.1.

@ST-DDT
Copy link
Member

ST-DDT commented Dec 5, 2023

@xDivisionByZerox I apologize I am new to this issue making and checked the section on "creating a PR for issue" on mistake.

No problem at all. If you have a suggestion for an algorithm that might fix this issue feel free to post it here anyway.

@ST-DDT
Copy link
Member

ST-DDT commented Dec 5, 2023

@ST-DDT Yes it is. Looks like it existed in 8.0.2, 8.2.0, and I did test it out using 8.3.1 locally. I will try to see if I can produce reproduction code using 8.3.1.

I don't think we changed anything significant in the method in this major version, so the problem is likely still there.
But thank you for your help.

@matthewmayer
Copy link
Contributor

matthewmayer commented Dec 6, 2023

Definitely a JS floating point issue. Not all floats can be exactly represented in Javascript.

if (precision !== undefined) {
      if (precision <= 0) {
        throw new FakerError(`Precision should be greater than 0.`);
      }

      const factor = 1 / precision;
      const int = this.int({
        min: min * factor,
        max: max * factor,
      });
      return int / factor;
    }

The code const factor = 1 / precision gives a non-integer for certain values of precision:

> 1/0.1
10
> 1/0.01
100
> 1/0.001
1000
> 1/0.0001
10000
> 1/0.00001
99999.99999999999
> 1/0.000001
1000000
> 1/0.0000001
10000000
> 1/0.00000001
100000000
> 1/0.000000001
999999999.9999999
> 1/0.0000000001
10000000000

@matthewmayer
Copy link
Contributor

You could do some kind of special case for precisions which are powers of 10 (most common case), like:

const logPrecision = Math.log10(precision)
if (Number.isInteger(logPrecision)) {
   return parseFloat((int / factor).toFixed(-logPrecision));
} else {
   return int / factor;
}

@matthewmayer
Copy link
Contributor

matthewmayer commented Dec 8, 2023

Similarly you would still have issues with precisions which are not powers of 10 e.g.:

> [ ...new Set( Array.from({ length: 500 }, () => faker.number.float({ min: 0, max: 1, precision: 0.3, }) ) ), ].sort();
[ 0, 0.3, 0.6, 0.8999999999999999 ]

@ST-DDT
Copy link
Member

ST-DDT commented Dec 8, 2023

You could do some kind of special case for precisions which are powers of 10 (most common case), like:

const logPrecision = Math.log10(precision)
if (Number.isInteger(logPrecision)) {
   return parseFloat((int / factor).toFixed(-logPrecision));
} else {
   return int / factor;
}

This might also work for non power 10 cases.

const logPrecision = precision.toString().replace(/\d+\./, '').length;
return parseFloat((int / factor).toFixed(logPrecision));

Not sure about the performance implications though.

@matthewmayer
Copy link
Contributor

Another option might be if 1/precision is "almost" an integer (within a small epsilon value) then we round it to an integer

@clocke3
Copy link
Author

clocke3 commented Dec 15, 2023

Any movement on this in development or still brainstorming?

@matthewmayer
Copy link
Contributor

Still under discussion. As a workaround,

parseFloat(faker.number.float({precision:0.00001, min:X, max:Y}).toFixed(5)) should do what you want

@matthewmayer
Copy link
Contributor

I think i have a fairly neat solution in #2581 - please take a look

@clocke3 clocke3 changed the title Generating a float with precision of 0.00001 give larger precision number Generating a float with precision of 0.00001 gives larger precision number Jan 19, 2024
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 has workaround Workaround provided or linked 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