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

Unit order is not preserved #624

Closed
hasselmm opened this issue Oct 12, 2024 · 17 comments
Closed

Unit order is not preserved #624

hasselmm opened this issue Oct 12, 2024 · 17 comments

Comments

@hasselmm
Copy link

Unit order is not preserved (or it's not obvious for the naive user how to achieve this).

Please consider this minimal example (https://godbolt.org/z/G8aG8c8hz):

#include <mp-units/core.h>
#include <mp-units/systems/si.h>

#include <print>

using namespace mp_units::si::unit_symbols;

inline constexpr auto kWh = kW * h;

int main()
{
    std::println("{}", 123 * kWh);
    std::println("{}", 456 * kW * h);
    return 0;
}

Expected output:

123 kW h
456 kW h

Or even:

123 kWh
456 kWh

Actual output:

123 h kW
456 h kW
@mpusz
Copy link
Owner

mpusz commented Oct 12, 2024

Unfortunately, I do not know how to preserve the unit order here 😢 We have to simplify the expression templates, and to do this, we have to sort the types in some arbitrary order. More info on this can be found in our docs.

Also, the text output is not meant for scientific usage. Some people even call it a "debug text output". Otherwise, we should probably provide LaTeX and HTML generators as well, which is far beyond the scope of such unit libraries.

@chiphogg
Copy link
Collaborator

Broadly speaking, my assessment is that "the juice isn't worth the squeeze". If we were determined to do this, I think we could figure out a way, but I expect the cost of the extra complexity would outweigh the benefit of printing unit symbols in a more familiar order.

In fact --- is order preservation really what you're looking for? Or is it that you want certain combinations of units to show up in a particular expected order, no matter which order they appear as inputs? Wouldn't you, as a human reader, prefer that (2 * h) * ((228 * kW) to produce 456 kW h, rather than 456 h kW?

If there were some single, global ordering for a subset of units, such that symbols for units earlier in the list were always written before symbols later in the list, then I think it wouldn't be too bad to do this. We could define the canonical ordering to use this ordering first (when available), and to use whatever-we're-using-now as a tiebreaker.

Meanwhile, there's a reasonable workaround. You can define a custom unit, kilowatt_hour, with the kWh symbol. I believe this will participate in every computation in just the same way as if it were a simple ad hoc combination of kW and h. But if you use this new unit for your quantity types, and in particular whenever printing, then you'll see the unit label you expect: https://godbolt.org/z/s7GbKzrzv

@mpusz
Copy link
Owner

mpusz commented Oct 12, 2024

If there were some single, global ordering for a subset of units, such that symbols for units earlier in the list were always written before symbols later in the list, then I think it wouldn't be too bad to do this. We could define the canonical ordering to use this ordering first (when available), and to use whatever-we're-using-now as a tiebreaker.

Unfortunately, even international standards like SI are explicitly inconsistent here 😞

@hasselmm
Copy link
Author

My naive expectation was that definition order would be preserved. Exactly for the reason that proper unit order might be domain specific. Anyway: I totally see the benefit of normalizing unit order. Even more named_unit is easy enough to use.

Maybe it's worth to mention normalized unit order at some more basic section of the documentation: Now that Mateusz pointed me to the releated docs I feel rather stupid that I asked. I have to admit that I really didn't care enough about implementation details to even consider reading that section before.

One more thing @mpusz: When you call these unit strings "debug text output", do you refer to the automatically generated text, or also to the explicitly named units? Asking as I tried to add wchar_t (or even char16_t) support for use with Qt, and in contrast to #557 being marked as "good first issue" this seems like pretty complex code with impressively deep stacktraces.

@Spammed
Copy link

Spammed commented Oct 13, 2024

That already works:

inline constexpr struct kilowatt_hour final :
named_unit<“kWh”, kW * h> {} kilowatt_hour;
inline constexpr auto kWh = kilowatt_hour;

std::println("{}", (456 * kW * h).in(kWh)); // -> "456 kWh"

But I would also like it to be part of the formatting task (e.g. for the purpose of i8n or user preferences)
Something along those lines:

std::println(“{::in(kWh)}”, 456 * kW * h); // -> "456 kWh"

@mpusz
Copy link
Owner

mpusz commented Oct 13, 2024

Maybe it's worth to mention normalized unit order at some more basic section of the documentation

That is a good idea. I will do it soon. Probably I will add a short note about it next to units introduction and will put a longer description in FAQ. Thanks!

Even more named_unit is easy enough to use.

I will add this to "Use cases" or FAQ as well.

Now that Mateusz pointed me to the releated docs I feel rather stupid that I asked.

Don't be. At my C++ workshops I always say there are no stupid questions 😉 It is good that you asked because we want to know what may be the expectations of our users.

@mpusz
Copy link
Owner

mpusz commented Oct 13, 2024

When you call these unit strings "debug text output", do you refer to the automatically generated text, or also to the explicitly named units?

Actually, that is not me who invented and use this term. That is why our docs do not have "Debug" next to the "Text Output" chapter 😉 I've heard this term in the ISO C++ Committee. They are partially right, though. This is mostly used to ensure/present that the program works as expected. Something like printf-like debugging.

@mpusz
Copy link
Owner

mpusz commented Oct 13, 2024

Asking as I tried to add wchar_t (or even char16_t) support for use with Qt, and in contrast to #557 being marked as "good first issue" this seems like pretty complex code with impressively deep stacktraces.

I am not sure what the Qt constraints are here. By this issue #557, I meant to provide support for std::wcout and std::format() taking [std::wformat_string](http://en.cppreference.com/w/cpp/utility/format/basic_format_string)<Args...>.

You should not need to change anything else. There is no need to change named_unit or symbol_text in particular.

To provide a properly encoded input to wcout and format (and possibly for char16_t if you need it), a runtime text conversion from char8_t or portable char text should be performed.

@mpusz
Copy link
Owner

mpusz commented Oct 14, 2024

But I would also like it to be part of the formatting task (e.g. for the purpose of i8n or user preferences)
Something along those lines:

std::println(“{::in(kWh)}”, 456 * kW * h); // -> "456 kWh"

@Spammed, if I understand it correctly, you would like to use this for internationalization, which can vary on runtime depending on the user settings. However, a unit conversion requires a destination unit to be known at compile time.

I assume you want the framework just to use the text provided, but I wouldn't do that without verification. To verify, we would need to find a way to convert this textual representation to the actual type kWh (which I really do not know how to do with current C++ rules). After that, we would need to check if we can convert from the current to the given unit, followed by a potential quantity value scaling to a new unit.

Taking the above into account, I do not think it is doable. So, we probably have to stick with the current syntax:

std::println(“{}”, (456 * kW * h).in(kWh)); // -> "456 kWh"

Please let me know if I am missing something here.

@Spammed
Copy link

Spammed commented Oct 14, 2024

...if I understand it correctly, you would like to use this for internationalization, which can vary on runtime depending on the user settings.

Yes, that would be one of my use cases.

I assume you want the framework just to use the text provided,

No, not just to.

I maintain an older structural analysis program in which the user can switch at runtime back and forth between different units (m, cm, mm, inches, feed, N, pond, lp, rad, °, etc.) on the one hand and between English and German on the other and the display updates itself accordingly. To make things a little more complicated, the user can select different units and languages for output on 'paper' and output on screen.

To minimize my (programmers) confusion, ALL data is ALWAYS stored and handled internally in SI units (without any 'quantity' intelligence). So that different units are 'only' a problem of input and output.

I am following your efforts with great interest and hope that one day the compiler will point out my physically nonsensical expressions :-).
If such a framework also presents the units 'nicely' (depending on runtime decisions), that would of course also be very welcome.

@mpusz
Copy link
Owner

mpusz commented Oct 14, 2024

I'm sorry, but I think we can't do much for your use case without #483.

@Spammed
Copy link

Spammed commented Oct 14, 2024

Don't feel sorry. Powerful compiletime functionality has immense added value and runtime features can be added on top. I look forward to what may come.
(#483 is an interesting read)

@Spammed
Copy link

Spammed commented Oct 16, 2024

@mpusz: I have seen your new FAQ note about, the topic:

[...] 42 * kWh / (2 * h) will result with 21 kWh/h

OK. Then, it's feels better to me to use '.in()' on every output and "kWh" only for that purpose.

std::println("{}", (42 * kW * h).in(kWh)); // -> "42 kWh"
std::println("{}", (42 * kW * h / (2 * h)).in(Wh)); // -> "21 Wh"

(So I still see it purely as a question of output.)

@mpusz
Copy link
Owner

mpusz commented Oct 16, 2024

The second line seems wrong. Did you mean:

std::println("{}", (42 * kW * h / (2 * h)));  // -> "21 kW"

@Spammed
Copy link

Spammed commented Oct 16, 2024

Sorry, yes.

@mpusz
Copy link
Owner

mpusz commented Oct 16, 2024

Sorry, yes.

Notice that you do not need to convert the unit in the second case.

So I still see it purely as a question of output

The generated unit type will also be more user-friendly.

@mpusz
Copy link
Owner

mpusz commented Oct 16, 2024

As stated above, we can't do anything to mirror the multiplication order in the units order.

I extended the documentation to clearly state it so other users will not be confused (if they read the docs 😉):

With the above, I think we can close this comment.

If you have thoughts on improving here, please reopen and share your ideas.

@mpusz mpusz closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2024
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

4 participants