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

Floating point when using multipleOf #379

Closed
adelura opened this issue Feb 22, 2018 · 10 comments · Fixed by #675
Closed

Floating point when using multipleOf #379

adelura opened this issue Feb 22, 2018 · 10 comments · Fixed by #675

Comments

@adelura
Copy link

adelura commented Feb 22, 2018

Hi, I just started to play with your library and noticed one thing:

let jsf = require('json-schema-faker');
let seed = [ 0.25546874766962246, 0.06559633646612273 ];
jsf.option({ random: () =>seed.shift() });

let schema = {
  "type": "number",
  "multipleOf": 0.01
};

jsf.resolve(schema).then(function(sample) {
  console.log(sample); // 13119267.290000001
});

What I expect here is two decimal places at most. I can imagine that you might be aware of that as this project looks mature, but I couldn't find any issue related to that.

@pateketrueke
Copy link
Member

Nice issue, you're totally right, it should generate a two-decimal number instead.

So much thanks for your feedback, I'll be fixing this soon, if you found something else please let us know. 😉

@pateketrueke
Copy link
Member

pateketrueke commented Sep 17, 2018

This is getting complex: mafintosh/is-my-json-valid#172

I'm trying to understand this properly, using this validator: https://json-schema-validator.herokuapp.com/

[ {
  "level" : "error",
  "schema" : {
    "loadingURI" : "#",
    "pointer" : ""
  },
  "instance" : {
    "pointer" : ""
  },
  "domain" : "validation",
  "keyword" : "multipleOf",
  "message" : "remainder of division is not zero (13119267.290000001 / 0.01)",
  "value" : 13119267.290000001,
  "divisor" : 0.01
} ]

The value 13119267.290000001 is not valid, but 0.94 is, of course, multiple of 0.01 right?

It depends on how validators treat this: https://runkit.com/pateketrueke/5b9ef02f3cede20011d5f06f

The results are weird, seriously!

  • is-my-json-schema invalidates the long .290000001 remainder
  • tv4, ajv and z-schema both invalidates 0.94 as multipleOf: 0.01 ... why?

We'll try to generate an appropriate value for multipleOf as long validators don't get it wrong.

I think rounding up 13119267.290000001 to 13119267.29 is the best choice and it's easier to solve.

Any thoughts?

pateketrueke added a commit that referenced this issue Sep 26, 2018
* Lock deps

* Run npm audit fix

* Remove old deps; cleanup

* Fix package; remove types from sources

* Delete old typescript sources; cleanup

* Move to modern testing and libs; cleanup

* Remove ancient build scripts and lib; cleanup

* Upgrade ESLint rules

* Fixed schema tests; cleanup

* Fixed scripts for CI

* Watch more extensions; cleanup

* Apply changes from PR #432

* Fix #458

* Add test for #455

* Make ESLint happier; take 1

* Make ESLint happier; take 2

* Add is-my-json-valid to validators

* Use Object.assign() instead; fix

* Add tests for #456; cleanup

* Add ajv to validators; remove jayschema

* Fix #453; handle single-prop cases

* Add fixedProbabilities option to handle random vs fixed cases; fix #443

* Add tests for #446; cleanup

* Ignore properties.default; fix #442

* Add tests for #425; fixed

* Add tests for #427; fixed

* Fill up with patternProperties; fix #416

* Add support for built-in templates; fix #419

* Add tests for #408; wont fix

* Merge references into schemas; fix #345

* Fill missing properties with required ones; fix #369

* Support for seeding on tests; add #379 tests

* Improve enum support for other types; fix #405

* Query and pick for random items; fix #400 #429

* Enable resolveJsonPath by default; fix #407

* Add methods for clearing out containers; fix #440

* Remove deref from dependencies; fixing refs issues

* Fix resolve order; await on tests

* Cleanup

* Add tests for #386; pending

* Add deprecation logs; cleanup

* Exclude external deps to get ~27KB dist, bundle is ~413KB; fix

* Call .resolve() on tests; fix

* Fix built filenames; cleanup

* Revert "Enable resolveJsonPath by default; fix #407"

This reverts commit 48b67ab.

* Fix exported name as JSONSchemaFaker

* Add missing useExamplesValue() from #441; fix

* 0.5.0-rc16

* Fix tests on CI

* Use Object.assign() instead; fix

* Make tests run without async/await; fix

* Apply fixedProbabilities on arrays; fix #456

* Increase timeout on CI

* Not used anymore

* Cleanup; WIP

* Moving and updating docs; WIP

* Improve docs

* Enable ignoreMissingRefs flag support; fix

* Typo

* Force typecasted type fo fix #467

* Fix bullets

* Build dist files

* Fixed tests

* Test for custom typecasting; fix

* Run all tests; fix
@Jeffersondyj
Copy link
Contributor

Hello,I just found another problem when multipleOf is float

let jsf = require('json-schema-faker');

let schema = {
    "type": "number",
    "multipleOf": 0.01,
    "minimum": 7,
    "maximum": 10
};

jsf.resolve(schema).then(function(sample) {
    console.log(sample);
});

It was expected to be a number between 7 and 10, just like 7.01, or 9.99
but it turn out to be a number between 0 and 3, just like 0.52, 2.88

Then I found the source code: https://github.com/json-schema-faker/json-schema-faker/blob/master/src/lib/types/number.js

line38 to line50, in line50, should return min + num instead of return num

And in this ut, https://github.com/json-schema-faker/json-schema-faker/blob/master/tests/schema/core/issues/issue-252.json
when minimum is zero, the ut will be passed. So maybe we should add a ut when minimum is not zero

Thank you very much for your replying.

@pateketrueke
Copy link
Member

Interesting issue, feel free to change the code as you think and submit it as PR!

I'll be checking this as soon as I can, any help is very appreciated.

@GaryStrange
Copy link

Hi, was the original issue resolved?

I just ran a schema through the online .org site and it didn't generate fake values that were faithful to my multipleOf definitions.

Thanks,
Gary

@pateketrueke
Copy link
Member

Hi @GaryStrange I think it is solved, but you know, there could be regressions.

¿Can you share the schema and output you got? Thank you!

@GaryStrange
Copy link

It appears to be non-deterministic so if you have enough 'number' attributes you can increase your chances of exposing the problem.

enter...

{
"QTYORDERED": {
"type": "number",
"multipleOf": 1e-7
},
"QTYORDERED2": {
"type": "number",
"multipleOf": 1e-7
},
"QTYORDERED3": {
"type": "number",
"multipleOf": 1e-7
},
"QTYORDERED4": {
"type": "number",
"multipleOf": 1e-7
},
"QTYORDERED5": {
"type": "number",
"multipleOf": 1e-7
}
}

into

https://json-schema-faker.js.org/

and you should see at least on fake with more than 7 d.p

@pateketrueke
Copy link
Member

Woah, you're using 1e-7 as a multiple, do you think is a common case?

@lucas-weje
Copy link

lucas-weje commented Aug 16, 2021

Hi,

I am experiencing the same problem as @GaryStrange with the multipleOf definitions.

Currently working with SAP data types, and you can see a snippet of my schema below:
image

Using jsf.generate() on the schema I am then returned this:
image

Here you can see 'CDEC31_0' does not conform to the multipleOf = 1.

Hope this makes sense, and that you can help :)

@pateketrueke
Copy link
Member

Got it, I think we could spent some time rewriting a simple solution for multipleOf, then integrate it back to JS, what do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants