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

Implicit construction of quantities from a value #410

Closed
mpusz opened this issue Dec 21, 2022 · 6 comments
Closed

Implicit construction of quantities from a value #410

mpusz opened this issue Dec 21, 2022 · 6 comments

Comments

@mpusz
Copy link
Owner

mpusz commented Dec 21, 2022

With the new design a following change was needed:

using state = kalman::state<quantity<isq::position_vector[m]>, quantity<isq::velocity[m / s]>>;
-const state initial = {30 * km, 40 * (m / s)};
+const state initial = {30 * isq::position_vector[km], 40 * isq::velocity[m / s]};

As @JohelEGP correctly noticed that:

seems like an unfortunate deviation from the general idea of ISO 80000-1, 7.1.4 “Expression for quantities” and 7.2 “Names and symbols for units”'s 7.2.1 “General”. Since state is already strongly typed, could the previous iterator be made to work? I think it's preferable if C++ code looked like the quantity expression. Strong typing could be added without affecting it.

The intention seems to be for "30 km" to not be decorated with anything other than the number and the unit. In C++, we use * out of necessity. But this shouldn't be the place to specify the quantity.

I was thinking of permitting x * km, where x is a function parameter, to initialize any quantity of dimension L (besides the requirements on the representation and the unit).

Discussion of alternatives

A number and a unit are not enough to define a quantity. Even adding a dimension was found to be deficient. This was one of the biggest issues with the old "references".

Regarding function parameters, I am not the biggest fan of this syntax in general. Consider:

void foo(double a, double b)
{
  quantity<isq::distance[km]> l = a * m;
  quantity<isq::duration[h]> d = b * s;
  quantity<isq::speed<km / h>> speed = l / d;
}

This looks really cryptic to me and might be a source of confusion. I would much prefer people to type:

void foo(double a, double b)
{
  auto l = a * isq::distance[m];
  auto d = b * isq::duration[s];
  quantity<isq::speed<km / h>> speed = l / d;
}

So I do not think that there is a problem here. Additionally, to support such a scenario, we will have to introduce yet another type/abstraction that would handle such a scenario and make a quantity implicitly constructible from it. I am not so sure if it is worth it.

Another point here worth considering is that right now, we do the following to define a scaled unit:

inline constexpr struct minute : named_unit<"min", mag<60> * second> {} minute;

and I hope that if expression aliases became a part of the C++ language, we will write:

inline constexpr struct minute : named_unit<"min", 60 * second> {} minute;

so a 60 * s is just yet another scaled unit and not a quantity value.

Proposed solution

However, there is another even simpler solution to the problem. For now, the quantity cannot be implicitly constructible from the representation type, so we cannot just leave numbers to initialize the struct.

I believe that the creation of quantities from raw number arguments is quite rare and happens only on the boundaries of strongly typed engines. On the other hand, the initialization of some quantities from constant values will happen quite often, and sometimes, a quantity will be packed inside of the aggregate. Until now, we were not able to initialize such aggregates in an efficient way, and now with the V2 design it looks like this:

quantity<isq::length[m]> vec[3] = { 1 * isq::length[m], 2 * isq::length[m], 3 * isq::length[m] };
std::tuple<quantity<isq::length[m]>, quantity<isq::time[s]>> t = { 1 * isq::length[m], 1 * isq::time[s] };

If we would make the constructor implicit, then the above could be rewritten as:

quantity<isq::length[m]> vec[3] = { 1, 2, 3 };
std::tuple<quantity<isq::length[m]>, quantity<isq::time[s]>> t = { 1, 1 };

which I think could be a big improvement here.

On the other hand, it would allow something like:

void foo(quantity<isq::length[m]>);

int main()
{
  foo(3);
}

which I am not so sure if it is a great idea, although I am ready to discuss and consider it.

Any ideas, thoughts, or comments are welcome here.

@mpusz mpusz mentioned this issue Dec 21, 2022
18 tasks
@JohelEGP
Copy link
Collaborator

Beginning of the thread: #391 (comment).

The last point, "proposed solution", which is about sum types of quantities, feels very much like

quantity<dim_length, metre> f() {
  return {1}; // error: constructor is `explicit`
  return quantity<dim_length, metre>{1}; // OK
}

@mpusz
Copy link
Owner Author

mpusz commented Dec 21, 2022

Yes, that is a really good point! Another reason to consider such a change.

@JohelEGP
Copy link
Collaborator

JohelEGP commented Dec 21, 2022

It can't be done while disallowing f(3). If quantity were an aggregate, it would work to some extent, requiring LLVM 16 for some use-cases (llvm/llvm-project#54040 (comment)), and with some downsides (https://quuxplusone.github.io/blog/2022/06/03/aggregate-parens-init-considered-kinda-bad/). f({3}) would still work.
1671650496
1671650507

@mpusz
Copy link
Owner Author

mpusz commented Dec 21, 2022

Yes, I know. But the more I think about it, the more I am convinced that f(3) is not that bad and the benefits outweigh the possible issues. I hope to hear others' opinions on this as well, and then we can decide if we should do this change or not.

@burnpanck
Copy link
Contributor

I tend to disagree here. To me, the main reason for using strongly typed engines is to avoid errors due to mis-interpretation between different units. These errors are of the worst kind, because they may be extremely hard to find. Because of that, I would prefer if the compiler would complain loudly whenever I separate units from values without having a clear and obvious marker (i.e. at the call site) that units are being converted.

(For the same reason, I always prefer .number_as<some_unit>() magnitude accessors over .number() - style accessors. However, in this case, at least I have a method call that is comparatively easy to audit).

Is there a way we could make implicit conversions opt-in at the call-site?

auto vec = implicit_quantities<quantity<isq::length[m]>[3]>({1,2,3});
auto t = implicit_quantities<std::tuple<quantity<isq::length[m]>, quantity<isq::time[s]>>>({1,1});

This would probably require explicit specialisations for all classes of containers that should be supported, but to me, that would still be preferable.

@mpusz
Copy link
Owner Author

mpusz commented Jun 15, 2023

Done in V2.

@mpusz mpusz closed this as completed Jun 15, 2023
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

3 participants