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

Solve most [UNLABELED_UNIT] instances by generating magnitude labels #85

Open
chiphogg opened this issue Feb 6, 2023 · 1 comment
Open
Labels
⬇️ affects: code (implementation) Affects implementation details of the code 📁 kind: enhancement New feature or request 💪 effort: medium
Milestone

Comments

@chiphogg
Copy link
Contributor

chiphogg commented Feb 6, 2023

Right now, the unit resulting from scaling a unit by a magnitude gets the default label, [UNLABELED_UNIT]. It's important that it not have the same label as the unscaled unit. Still, we can do much better if we design magnitude labels: automatic string constants that represent magnitudes.

Possible examples:

  • "1 / 18"
  • "Pi / 180"
  • "5,280"
  • "(3 * sqrt(3)) / (5 * Pi)"

To solve this, we would need to come up with a collection of canonical examples we can use as acceptance tests, and work out the general rules. We also need to figure out how to handle things like parentheses, powers, and roots.

@chiphogg
Copy link
Contributor Author

Based on discussion on the discord DMs for the C++ standard units library, I think a scaled unit should have the format [M U], where M is the magnitude label, and U is the label for the unscaled unit. That is: the new label should be fully enclosed in square brackets.

chiphogg added a commit that referenced this issue Nov 27, 2024
Think of this as "just enough of #85 to unblock #105".  The fact that
our common unit labels don't tell you the _size_ of the unit (#105) has
really, really been bugging me for a long time.  Recently, I realized we
don't need to do all of #85 to get it!  Instead, all we need to do is:

1. Build a _mechanism_ that we can easily _extend_.

2. Cover the most important use cases.

This PR creates the `MagnitudeLabel` trait mechanism (also accessible
via a function/value interface as `mag_label`).  We enumerate the
various categories of magnitudes that we can label, defaulting to
"unsupported".  The first two supported categories are _integers_ (that
fit in `std::uintmax_t`), and _rationals_.

We also add a trait, `has_exposed_slash`, looking forward to the obvious
use case of auto-generating labels for scaled units.  Those labels will
have the form `"[M U]"` for a unit of label `"U"` scaled by a magnitude
of label `"M"`.  If `has_exposed_slash` is `true` for a given magnitude
label, then we'll know to make this `"[(M) U]"` instead.

Finally, we move a couple of `StringConstant`-ish utilities into
`"string_constant.hh"`, so that we can use them in our implementation.

Helps #85.
chiphogg added a commit that referenced this issue Nov 29, 2024
Think of this as "just enough of #85 to unblock #105".  The fact that
our common unit labels don't tell you the _size_ of the unit (#105) has
really, really been bugging me for a long time.  Recently, I realized we
don't need to do all of #85 to get it!  Instead, all we need to do is:

1. Build a _mechanism_ that we can easily _extend_.

2. Cover the most important use cases.

This PR creates the `MagnitudeLabel` trait mechanism (also accessible
via a function/value interface as `mag_label`).  We enumerate the
various categories of magnitudes that we can label, defaulting to
"unsupported".  The first two supported categories are _integers_ (that
fit in `std::uintmax_t`), and _rationals_.

We also add a trait, `has_exposed_slash`, looking forward to the obvious
use case of auto-generating labels for scaled units.  Those labels will
have the form `"[M U]"` for a unit of label `"U"` scaled by a magnitude
of label `"M"`.  If `has_exposed_slash` is `true` for a given magnitude
label, then we'll know to make this `"[(M) U]"` instead.

Finally, we move a couple of `StringConstant`-ish utilities into
`"string_constant.hh"`, so that we can use them in our implementation.

Helps #85.
chiphogg added a commit that referenced this issue Nov 29, 2024
First, when multiplying a `ScaledUnit` by another magnitude, we now fold
it into the existing magnitude of the scaled unit.  Previously, we'd end
up with `ScaledUnit<ScaledUnit<U, M1>, M2>`, and so on.  We also now
omit any "trivial" scaling factors, whether because we're scaling by
`mag<1>()`, or (more commonly) whether we've applied a bunch of
different scale factors and they all cancel out.

(We did need to tweak a few cases that were relying on `U{} * mag<1>()`
being meaningfully different from `U{}`.)

Next, we now auto-generate labels for `ScaledUnit` specializations.  For
`ScaledUnit<U, M>`, if `U` has label `"U"`, and `M` has label `"M"`, we
generate a label `"[M U]"` --- or, if `"M"` contains an exposed slash
`"/"`, we'll generate the label `"[(M) U]"` for lack of ambiguity.  This
resolves the vast majority of `[UNLABELED_UNIT]` labels.  The remaining
work on #85 is simply to generate labels for a wider variety of
magnitude label categories.

Finally: we formerly had no way to decide ordering between units that
are _both_ specializations of `ScaledUnit`, which _do_ have identical
dimension _and_ magnitude, and yet are _not_ the same unit.  (For
example, something like `"[(1 / 4) ft]"` and `"[3 in]"`.)  This may have
been somewhat obscure in the past, but with the upcoming work on #105,
it's about to become very common.  We added a test case that exposes
this, and then updated our ordering code to handle this case.

Helps #85.  Unblocks #105.
chiphogg added a commit that referenced this issue Dec 2, 2024
First, when multiplying a `ScaledUnit` by another magnitude, we now fold
it into the existing magnitude of the scaled unit.  Previously, we'd end
up with `ScaledUnit<ScaledUnit<U, M1>, M2>`, and so on.  We also now
omit any "trivial" scaling factors, whether because we're scaling by
`mag<1>()`, or (more commonly) whether we've applied a bunch of
different scale factors and they all cancel out.

(We did need to tweak a few cases that were relying on `U{} * mag<1>()`
being meaningfully different from `U{}`.)

Next, we now auto-generate labels for `ScaledUnit` specializations.  For
`ScaledUnit<U, M>`, if `U` has label `"U"`, and `M` has label `"M"`, we
generate a label `"[M U]"` --- or, if `"M"` contains an exposed slash
`"/"`, we'll generate the label `"[(M) U]"` for lack of ambiguity.  This
resolves the vast majority of `[UNLABELED_UNIT]` labels.  The remaining
work on #85 is simply to generate labels for a wider variety of
magnitude label categories.

Finally: we formerly had no way to decide ordering between units that
are _both_ specializations of `ScaledUnit`, which _do_ have identical
dimension _and_ magnitude, and yet are _not_ the same unit.  (For
example, something like `"[(1 / 4) ft]"` and `"[3 in]"`.)  This may have
been somewhat obscure in the past, but with the upcoming work on #105,
it's about to become very common.  We added a test case that exposes
this, and then updated our ordering code to handle this case.

Helps #85.  Unblocks #105.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⬇️ affects: code (implementation) Affects implementation details of the code 📁 kind: enhancement New feature or request 💪 effort: medium
Projects
None yet
Development

No branches or pull requests

1 participant