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

Slug definition appears to be off by a few parts per billion #289

Open
chiphogg opened this issue Jun 1, 2022 · 2 comments
Open

Slug definition appears to be off by a few parts per billion #289

chiphogg opened this issue Jun 1, 2022 · 2 comments
Assignees
Labels

Comments

@chiphogg
Copy link

chiphogg commented Jun 1, 2022

Please include the following information in your issue:

  1. Which version of units you are using: 2.3.1, although this problem exists on all versions, including 3.x.

  2. Which compiler exhibited the problem (including compiler version): All of them: it's a wrong definition in the code.


A slug is defined as the mass accelerated by 1 ft/s^2 by a net force of one lbf. This works out to some exact number of kilograms, which we can find using the following other exact definitions:

  • Standard gravitational acceleration is exactly 9.80665 m/s^2.
  • 1 foot is exactly 0.3048 m.
  • 1 pound mass is exactly 0.45359237 kg.

When I simplify this, I get that a slug is exactly (8896443230521 / 609600000000) kg, which works out to 14.5939029372063648293963... (etc.).

However, the units library slug is defined as exactly 14.5939029 kg. This amounts to an error of a few parts per billion. The error propagates to related units, e.g., foot-pounds of torque.

I think you could fix this: the numerator and denominator both fit into a std::intmax_t for std::ratio. However, since you're using std::ratio to represent unit magnitudes, instead of a vector space representation, you'll probably be more vulnerable to overflow when defining related units.

@chiphogg
Copy link
Author

chiphogg commented Jun 2, 2022

slug_to_kg

Here's me showing the steps to make it easy for others to reproduce. I'm using only integers, and I believe they all trace back to exact definitions. The end result matches the definition in the library exactly before truncation, which adds to my confidence that I performed the computation correctly. The red-underlined digits are the ones omitted by the library.

@nholthaus
Copy link
Owner

Mr. Hogg,

Thank you for the issue. I think you're right and your analysis is correct.

The aim of units as opposed to other C++ dimensional analysis libraries is to be the most easy-to-use syntactically (including dreaded implicit conversions), and to be absolutely precise with unit definitions unless it's absolutely unusable due to overflow, etc. In this case, units does not meet that standard, so I'll put in the fix.

@nholthaus nholthaus self-assigned this Jun 2, 2022
@nholthaus nholthaus added the bug label Jun 2, 2022
chiphogg added a commit to aurora-opensource/au that referenced this issue Feb 20, 2023
This creates a file, `"compatibility/nholthaus_units.hh"`, which makes
it easy to set up a correspondence between each Au Quantity type, and
the corresponding nholthaus units type.  This "correspondence" includes
bidirectional implicit conversions: so, you can pass
`::au::QuantityD<Meters>` to an API expecting
`::units::length::meter_t`, and vice versa.

This correspondence is purely opt-in.  The file also needs to be
included _after_ both of the other two libraries are included.  Since
this is fragile in general, we give instructions and an example for how
to make it _not_ fragile, in `nholthaus_units_example_usage.hh`.

We include a variety of unit tests to show that the quantities are
indeed equivalent between the libraries.  (Actually, this isn't quite
always the case.  These tests uncovered a bug in the nholthaus library,
nholthaus/units#289.)

We reserve the documentation of the corresponding-quantity feature in
general, as well as its application to the nholthaus library, for a
future PR.
chiphogg added a commit to aurora-opensource/au that referenced this issue Feb 22, 2023
This creates a file, `"compatibility/nholthaus_units.hh"`, which makes
it easy to set up a correspondence between each Au Quantity type, and
the corresponding nholthaus units type.  This "correspondence" includes
bidirectional implicit conversions: so, you can pass
`::au::QuantityD<Meters>` to an API expecting
`::units::length::meter_t`, and vice versa.

This correspondence is purely opt-in.  The file also needs to be
included _after_ both of the other two libraries are included.  Since
this is fragile in general, we give instructions and an example for how
to make it _not_ fragile, in `nholthaus_units_example_usage.hh`.

We include a variety of unit tests to show that the quantities are
indeed equivalent between the libraries.  (Actually, this isn't quite
always the case.  These tests uncovered a bug in the nholthaus library,
nholthaus/units#289.)

We reserve the documentation of the corresponding-quantity feature in
general, as well as its application to the nholthaus library, for a
future PR.

Test plan
---------

- [x] Add new unit tests for this file.
- [x] Add example file that shows how to use it.
- [x] Test that the file can replace Aurora's existing implementation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants