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

Lacking precision necessary for required comparisons #132

Open
kyrofa opened this issue Feb 3, 2022 · 10 comments
Open

Lacking precision necessary for required comparisons #132

kyrofa opened this issue Feb 3, 2022 · 10 comments

Comments

@kyrofa
Copy link

kyrofa commented Feb 3, 2022

This test passes

it("should compare properly", function() {
  var galQty = Qty("1 gal");
  var qtQty = Qty("4 qt");
  expect(galQty.eq(qtQty)).toBeTrue();
});

However, for more complex units it fails:

it("should compare properly", function() {
  var galQty = Qty("1 gal/acre");
  var qtQty = Qty("4 qt/acre");
  expect(galQty.eq(qtQty)).toBeTrue();
});

This is due to a lack of precision. If you take a look at baseScalar in the latter test, you'll see that they're VERY close down to twenty decimal places or so, but then differ. These types of problems are generally solved by using a library like big.js, but that does mean taking on a dependency. How do you feel about that? A less-great option would be to check how close the values are in compareTo and claim that they're equal if they're "close enough" (this is how I'm working around this issue locally for now). Is there another more preferred way to solve this problem? Happy to do the legwork, just need to know what will be approved.

@rage-shadowman
Copy link
Contributor

Look at src/quantities/definitions.js. Their definitions aren't exact. They are defined in terms of cubic meter, and doing the math, quart is not 1/4 gallon.

I'm not sure which one is less precise, or how to make them precise, but going by the current definitions, they really are not equal.

@kyrofa
Copy link
Author

kyrofa commented Mar 18, 2022

They should be equal though, agreed?

@rage-shadowman
Copy link
Contributor

Yeah, the definition of a US gallon is 4 quarts. I'm just not sure how best to correct it, or what the true quantity would be in terms of cubic meters.

@kyrofa
Copy link
Author

kyrofa commented Mar 18, 2022

Yeah I think ideally these would be ratios (rationals), and then we'd use something like big.js to retain precision. I don't know where to find such magical ratios, though, if these are lacking precision.

@rage-shadowman
Copy link
Contributor

rage-shadowman commented Mar 18, 2022

Self correction:

I was wrong with the above. The definition of quart in the file is exactly 1/4 gallon. Double imprecision seems to be the real issue here. If we can't do something to make the numbers exact (not sure if defining based on whole number numerator/denominator rather than floating point numbers would help, and if so, how much work that would take), then maybe adding an optional precision argument to the equality comparison? (kinda like what you suggested as a compromise solution but keeps exactness checks intact if the argument is not supplied and allows user to determine how close is close enough)

@kyrofa
Copy link
Author

kyrofa commented Mar 18, 2022

Well, the fact that the first test passed indicates that we may have enough precision. But we start losing it when we start dividing down the units. If we converted these to rationals (even just starting with the decimal values, but turn them into reduced num/den combos), then used big.js from there, that could very well be enough. I didn't want to start down that path without buy-in, though, given how much pride the README takes in having no deps.

@kyrofa
Copy link
Author

kyrofa commented Mar 18, 2022

not sure if defining based on whole number numerator/denominator rather than floating point numbers would help

Not on its own, we'll still lose precision as we operate on them. We need to KEEP them as rationals as we operate, which is where big.js comes in.

@rage-shadowman
Copy link
Contributor

If that works, then IMO true precision would be great! But will that really work? Is everything truly definable using rational numbers like that? Or will we still end up needing some sort of precision argument in the equality checks? (I do see Math.PI used in some of the definitions)

@rage-shadowman
Copy link
Contributor

I didn't want to start down that path without buy-in, though, given how much pride the README takes in having no deps.

As far as "no deps" goes, I don't know what @gentooboontoo would say, but I don't think you'll have any arguments against adding a precision argument to the equality checks.

@kyrofa
Copy link
Author

kyrofa commented Mar 18, 2022

If that works, then IMO true precision would be great! But will that really work? Is everything truly definable using rational numbers like that?

Probably not all of them, but if we use ruby units for inspiration it seems like most of them.

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

No branches or pull requests

2 participants