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

Format general Magnitudes in printed output #386

Open
chiphogg opened this issue Aug 6, 2022 · 7 comments
Open

Format general Magnitudes in printed output #386

chiphogg opened this issue Aug 6, 2022 · 7 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers low priority Can wait a bit in the backlog
Milestone

Comments

@chiphogg
Copy link
Collaborator

chiphogg commented Aug 6, 2022

Now that we are using Magnitude, not ratio, we need to decide how a Magnitude should be printed, and then implement that policy. We'll need to take into account, among perhaps other considerations:

  • Factoring out powers of 10
  • Irrational factors, such as (powers of) pi
  • Radical powers of integer factors

I don't know what the policy should be, but step one is to figure that out, and step two is to implement it.

(I don't have time right now to work on this, but I thought it important to have an issue to track the pending work and provide a home for discussion.)

@mpusz
Copy link
Owner

mpusz commented Aug 7, 2022

I agree with the points above. The units should be printed in terms of powers of 10 rather than a power of 2 and 5. This is connected to #377, and actually, after the refactoring of the type also to be the power of 10, the printing part should be trivial.

@mpusz
Copy link
Owner

mpusz commented Aug 31, 2022

Both FMT and ostream approach should be checked. At least ostreams will fail to compile for pi-based units as they use as_ratio which requires is_rational.

@mpusz mpusz added this to the v2.3.0 milestone Jun 22, 2024
@mpusz mpusz self-assigned this Jun 22, 2024
@mpusz
Copy link
Owner

mpusz commented Sep 24, 2024

Also, the printing of units like l / 100km could be improved (similar to what Au does).

@mpusz mpusz modified the milestones: v2.3.0, v2.6.0 Sep 24, 2024
@chiphogg
Copy link
Collaborator Author

Also, the printing of units like l / 100km could be improved (similar to what Au does).

Or, to be fair, similar to what Au wishes that it does. 😅 (aurora-opensource/au#85)

@mpusz
Copy link
Owner

mpusz commented Oct 3, 2024

I've just implemented everything besides:

Radical powers of integer factors

Here are the unit tests:

// scaled units
static_assert(unit_symbol(mag<100> * metre) == "[100 m]");
static_assert(unit_symbol<usf{.encoding = ascii}>(mag<100> * metre) == "[100 m]");
static_assert(unit_symbol(mag<1000> * metre) == "[10³ m]");
static_assert(unit_symbol(mag_power<10, 3> * metre) == "[10³ m]");
static_assert(unit_symbol<usf{.encoding = ascii}>(mag<1000> * metre) == "[10^3 m]");
static_assert(unit_symbol(mag<6000> * metre) == "[6 × 10³ m]");
static_assert(unit_symbol(mag<6> * mag_power<10, 3> * metre) == "[6 × 10³ m]");
static_assert(unit_symbol<usf{.encoding = ascii}>(mag<6000> * metre) == "[6 x 10^3 m]");
static_assert(unit_symbol(mag<10600> * metre) == "[10600 m]");
static_assert(unit_symbol(mag<60> * second) == "[60 s]");
static_assert(unit_symbol(mag_ratio<1, 18> * metre / second) == "[1/18 m]/s");
static_assert(unit_symbol(mag_ratio<1, 18> * (metre / second)) == "[1/18 m/s]");
static_assert(unit_symbol(mag_ratio<1, 1800> * metre / second) == "[1/1800 m]/s");
static_assert(unit_symbol(mag_ratio<1, 1800> * (metre / second)) == "[1/1800 m/s]");
static_assert(unit_symbol(mag_ratio<1, 18000> * metre / second) == "[1/18 × 10⁻³ m]/s");
static_assert(unit_symbol(mag_ratio<1, 18000> * (metre / second)) == "[1/18 × 10⁻³ m/s]");
static_assert(unit_symbol<usf{.encoding = ascii}>(mag_ratio<1, 18000> * metre / second) == "[1/18 x 10^-3 m]/s");
static_assert(unit_symbol<usf{.encoding = ascii}>(mag_ratio<1, 18000> * (metre / second)) == "[1/18 x 10^-3 m/s]");
// magnitude constants
#if defined MP_UNITS_COMP_CLANG || MP_UNITS_COMP_CLANG < 18
inline constexpr struct e final : mag_constant<"e"> {
static constexpr long double value = std::numbers::e_v<long double>;
#else
inline constexpr struct e final : mag_constant<"e", std::numbers::e_v<long double> > {
#endif
} e;
static_assert(unit_symbol(mag<pi> * one) == "[𝜋]");
static_assert(unit_symbol<usf{.encoding = ascii}>(mag<pi> * one) == "[pi]");
static_assert(unit_symbol(mag<pi> * metre) == "[𝜋 m]");
static_assert(unit_symbol<usf{.encoding = ascii}>(mag<pi> * metre) == "[pi m]");
static_assert(unit_symbol(mag<2> * mag<pi> * metre) == "[2 𝜋 m]");
static_assert(unit_symbol<usf{.encoding = ascii}>(mag<2> * mag<pi> * metre) == "[2 pi m]");
static_assert(unit_symbol<usf{.separator = half_high_dot}>(mag<2> * mag<pi> * metre) == "[2⋅𝜋 m]");
static_assert(unit_symbol(mag<1> / mag<pi> * metre) == "[1/𝜋 m]");
static_assert(unit_symbol<usf{.encoding = ascii}>(mag<1> / mag<pi> * metre) == "[1/pi m]");
static_assert(unit_symbol<usf{.solidus = never}>(mag<1> / mag<pi> * metre) == "[𝜋⁻¹ m]");
static_assert(unit_symbol<usf{.encoding = ascii, .solidus = never}>(mag<1> / mag<pi> * metre) == "[pi^-1 m]");
static_assert(unit_symbol(mag<2> / mag<pi> * metre) == "[2/𝜋 m]");
static_assert(unit_symbol<usf{.encoding = ascii}>(mag<2> / mag<pi> * metre) == "[2/pi m]");
static_assert(unit_symbol<usf{.solidus = never}>(mag<2> / mag<pi> * metre) == "[2 𝜋⁻¹ m]");
static_assert(unit_symbol<usf{.encoding = ascii, .solidus = never}>(mag<2> / mag<pi> * metre) == "[2 pi^-1 m]");
static_assert(unit_symbol<usf{.solidus = never, .separator = half_high_dot}>(mag<2> / mag<pi> * metre) == "[2⋅𝜋⁻¹ m]");
static_assert(unit_symbol(mag<1> / (mag<2> * mag<pi>)*metre) == "[2⁻¹ 𝜋⁻¹ m]");
static_assert(unit_symbol<usf{.solidus = always}>(mag<1> / (mag<2> * mag<pi>)*metre) == "[1/(2 𝜋) m]");
static_assert(unit_symbol<usf{.encoding = ascii, .solidus = always}>(mag<1> / (mag<2> * mag<pi>)*metre) ==
"[1/(2 pi) m]");
static_assert(unit_symbol(mag_ratio<1, 2> / mag<pi> * metre) == "[2⁻¹ 𝜋⁻¹ m]");
static_assert(unit_symbol<usf{.solidus = always}>(mag_ratio<1, 2> / mag<pi> * metre) == "[1/(2 𝜋) m]");
static_assert(unit_symbol<usf{.encoding = ascii, .solidus = always}>(mag_ratio<1, 2> / mag<pi> * metre) ==
"[1/(2 pi) m]");
static_assert(unit_symbol(mag_ratio<1, 2> * mag<pi> * metre) == "[𝜋/2 m]");
static_assert(unit_symbol(mag_power<pi, 2> * one) == "[𝜋²]");
static_assert(unit_symbol<usf{.encoding = ascii}>(mag_power<pi, 2> * one) == "[pi^2]");
static_assert(unit_symbol(mag_power<pi, 1, 2> * metre) == "[𝜋^(1/2) m]");
static_assert(unit_symbol<usf{.encoding = ascii}>(mag_power<pi, 1, 2> * metre) == "[pi^(1/2) m]");
static_assert(unit_symbol(mag<pi> * mag<e> * one) == "[e 𝜋]");
static_assert(unit_symbol(mag<e> * mag<pi> * one) == "[e 𝜋]");
static_assert(unit_symbol<usf{.encoding = ascii}>(mag<pi> * mag<e> * one) == "[e pi]");
static_assert(unit_symbol(mag<pi> / mag<e> * one) == "[𝜋/e]");
static_assert(unit_symbol(mag<1> / mag<e> * mag<pi> * one) == "[𝜋/e]");
static_assert(unit_symbol<usf{.solidus = never}>(mag<pi> / mag<e> * one) == "[𝜋 e⁻¹]");
static_assert(unit_symbol(mag<e> / mag<pi> * one) == "[e/𝜋]");
static_assert(unit_symbol(mag<1> / mag<pi> * mag<e> * one) == "[e/𝜋]");
static_assert(unit_symbol<usf{.solidus = never}>(mag<e> / mag<pi> * one) == "[e 𝜋⁻¹]");
static_assert(unit_symbol(mag<1> / (mag<pi> * mag<e>)*one) == "[e⁻¹ 𝜋⁻¹]");
static_assert(unit_symbol<usf{.solidus = always}>(mag<1> / (mag<pi> * mag<e>)*one) == "[1/(e 𝜋)]");
static_assert(unit_symbol(mag<2> / (mag<pi> * mag<e>)*one) == "[2 e⁻¹ 𝜋⁻¹]");
static_assert(unit_symbol<usf{.solidus = always}>(mag<2> / (mag<pi> * mag<e>)*one) == "[2/(e 𝜋)]");

Please let me know it this is what you meant.

@mpusz mpusz added enhancement New feature or request good first issue Good for newcomers low priority Can wait a bit in the backlog labels Oct 3, 2024
@chiphogg
Copy link
Collaborator Author

chiphogg commented Oct 3, 2024

This is a terrific and industry-leading solution. (At least for C++; I don't know the state of the art elsewhere.)

There are maybe a few cases where I'm not completely sure what I'd like to see printed, like how it switches from 1/1800 to 1/18 × 10⁻³ when you go from 1/1800 to 1/18000. I haven't figured out under what circumstances I think we should reach for powers of 10 notation.

And, I shudder to think what we'll see for radical magnitudes, or for magnitudes that overflow intmax. 😂 I have given these use cases some thought, but the approach I plan to take for Au is exactly what I believe has been done here: provide answers for the core use cases first, and expand to other use cases only later as needed.

Awesome change!

@mpusz
Copy link
Owner

mpusz commented Oct 4, 2024

like how it switches from 1/1800 to 1/18 × 10⁻³ when you go from 1/1800 to 1/18000

constexpr std::intmax_t exp10 = _extract_power_of_10(ratio);
if constexpr (detail::abs(exp10) < 3) {
// print the value as a regular number (without exponent)
constexpr Magnitude auto num = _numerator(magnitude{});
constexpr Magnitude auto den = _denominator(magnitude{});
// TODO address the below
static_assert(ratio == num / den, "Printing rational powers not yet supported");
return detail::magnitude_symbol_impl<CharT, num, den, num_constants, den_constants, 0>(out, fmt);
} else {
// print the value as a number with exponent
// if user wanted a regular number for this magnitude then probably a better scaled unit should be used
constexpr Magnitude auto base = ratio / detail::mag_power_lazy<10, exp10>();
constexpr Magnitude auto num = _numerator(base);
constexpr Magnitude auto den = _denominator(base);
// TODO address the below
static_assert(base == num / den, "Printing rational powers not yet supported");
return detail::magnitude_symbol_impl<CharT, num, den, num_constants, den_constants, exp10>(out, fmt);
}

@mpusz mpusz modified the milestones: v2.6.0, v2.7.0 Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers low priority Can wait a bit in the backlog
Projects
None yet
Development

No branches or pull requests

2 participants