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

Optimize text output for scaled units #652

Open
mpusz opened this issue Nov 29, 2024 · 7 comments
Open

Optimize text output for scaled units #652

mpusz opened this issue Nov 29, 2024 · 7 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@mpusz
Copy link
Owner

mpusz commented Nov 29, 2024

Parenthesis for scaled units is needed only when more than one unit is used in the symbol of a derived unit. Otherwise, such grouping is not necessary. To make it clear where the runtime part of the quantity ends and the compile time unit symbol begins, an x separator can be used.

@mpusz mpusz added enhancement New feature or request good first issue Good for newcomers labels Nov 29, 2024
@chiphogg
Copy link
Collaborator

Could you give a simple example to illustrate? I think the issue would read a lot better with an example.

@mpusz
Copy link
Owner Author

mpusz commented Nov 29, 2024

Sure! Sorry, I should've done it right away, but due to the lack of time, I just made a short note for myself.

Here are some examples:

std::cout << 10 * (mag<100> * km) << "\n";
std::court << (10 * (mag<100> * km)).unit << "\n";
std::cout << 10 * (mag_ratio<1, 18000> * (m/s)) << "\n";
std::cout << 10 * (mag_ratio<1, 18000> * m/s) << "\n";
std::cout << 10 * km/h + 5 * m/s << "\n";
std::cout << 10 * L_per_100km << "\n";

Now:

10 (100 km)
(100 km)
10 (1/18 × 10⁻³ m/s)
10 (1/18 × 10⁻³ m)/s
140 EQUIV{(1/5 km/h), (1/18 m/s)}
10 L/(100 km)

Better:

10 × 100 km
100 km
10 × 1/18 × 10⁻³ m/s
10 (1/18 × 10⁻³ m)/s
140 EQUIV{1/5 km/h, 1/18 m/s}
10 L/(100 km)

To summarize:

  • if the unit of a quantity is a scaled_unit we do not put (...) (we do it if a scaled unit is a part of a derived_unit being printed)
  • if we print the entire quantity (and not just a unit), then we add × in between the unit and a number
  • additionally, we could consider if line 4 should not result with 10 × 1/18 × 10⁻³ m/s, but this requires even more logic and optimizations.

Please let me know your thoughts.

@chiphogg
Copy link
Collaborator

Personally, I'm partial to square brackets, and I'm partial to always having them be present.

I think 10 × 1/18 × 10⁻³ m/s looks a little strange, and 10 (1/18 × 10⁻³ m/s) just seems better to me.

Admittedly, 140 EQUIV{1/5 km/h, 1/18 m/s} is less cluttered than 140 EQUIV{(1/5 km/h), (1/18 m/s)}, which is nice.

As for the optimizations you mention, I think this would boil down to gathering all numeric scale factors into a single common one. In Au at least, this was really easy to get working locally, but I haven't put it out for review because I'm not sure I like the implications. Some seem clearly better, while others are a little bit worse --- for example, we'd lose the ability to have L / [100 km].

@Spammed
Copy link

Spammed commented Nov 30, 2024

In my humble opinion:
a) If square brackets are used, they should cover the entire unit.
b) Whether with or without square brackets should be a formatting option.
For example:
10 L/[100 km] looks wrong to me and
10 [L/100 km] misses the point.
So
10 L/(100 km) or
10 [L/(100 km)].

@mpusz
Copy link
Owner Author

mpusz commented Dec 1, 2024

LEWGI's feedback was that we already have too many formatting options. They continue to describe this as a debug text output, and for debugging, we do not have to support everything. They already suggested removing some of the options, but I have provided some pushback so far.

@drandrzejpalucki
Copy link

Same for me my friend :/

@chiphogg
Copy link
Collaborator

chiphogg commented Dec 3, 2024

Same for me my friend :/

Sorry, what was the same for you? I didn't understand what you were referring to.

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
Projects
None yet
Development

No branches or pull requests

4 participants