Replies: 35 comments 1 reply
-
|
I think the solution is to follow general C++ best practices, and avoid blanket using directives in favor of individual using declarations. That is, in each individual using mp_units::si::unit_symbols::m;...and similarly for every other symbol we want to incorporate. This has the advantage that it's easy to see where each symbol comes from, and of course also that it doesn't bring in a host of other symbols that we didn't think about. If we're in a file that has already used |
Beta Was this translation helpful? Give feedback.
-
|
As stated in https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2024/p3045r0.html#constructing-a-quantity:
In case "importing"
In the V2 design, we have plenty of options to choose from. |
Beta Was this translation helpful? Give feedback.
-
Maybe then best to rewrite the first example I came across to follow c++ best practice? |
Beta Was this translation helpful? Give feedback.
-
The ctor syntax I would recommend is the quantity{value,unit} , but then I find myself thinking, I might just as well do it in a terser syntax: #include <mp-units/systems/si/si.h>
#include <iostream>
#include <mp-units/format.h>
#include <mp-units/ostream.h>
#define MAKE_QUANTITY(Quantity, Numeric, Unit, Mp_unit) \
namespace Quantity {\
struct Unit : decltype( mp_units::quantity{ Numeric() , Mp_unit })\
{\
explicit Unit( Numeric v) : quantity{v, Mp_unit }{}\
};\
}
namespace pqs {
MAKE_QUANTITY(velocity ,double ,m_per_s ,mp_units::si::metre/mp_units::si::second )
MAKE_QUANTITY(time ,double ,s ,mp_units::si::second )
}
int main()
{
auto q1 = pqs::velocity::m_per_s{20};
auto q2 = pqs::time::s{2};
pqs::time::s q3 = q2;
std::cout << q1 * q2 <<'\n';
std::cout << q3 <<'\n';
return 0;
} |
Beta Was this translation helpful? Give feedback.
-
I do not think we have to rewrite the examples to use using-declarations. Using-directive is also fine in this case. In production code, we convert raw values to quantities in only a few places, typically only in a few functions. In such cases, it is often perfectly fine to use using-directives inside of such a function, and in case of naming conflicts, it doesn't hurt to spell the entire unit as we do not do this too often. In a production code, it is typically a mistake to use using-directives for the entire file scope, and we should nearly never do this inside of a header file. We have a lot of practice with An exception to this rule is typically unit tests, where we often create quantities from values. In such cases, using-directive is useful for the entire file as long as it does not produce hard-to-deal-with conflicts. |
Beta Was this translation helpful? Give feedback.
-
I played a lot with the syntax you proposed in V1 and spoke about it on conferences as well (https://youtu.be/Nt7B1vI8DtI?si=W_loEqQkageu5dHT&t=2329). There a few big issues with it where the most important ones are:
Each of the syntaxes I described in the V1 had quite a lot of issues. I believe that the solution I came up for V2 is much better from anything that we had before. |
Beta Was this translation helpful? Give feedback.
-
|
Composability of units https://github.com/kwikius/pqs/blob/master/examples/atmosphere.cpp#L17 and dimensions https://github.com/kwikius/pqs/blob/master/src/include/pqs/types/derived_quantity/acceleration.hpp#L9 were feature of pqs when mp-units V2 was a gleam in its authors eye I believe, with a similar syntax. I may be wrong about that but I seem to remember a variadic template and some heavyweight metaprogramming was required in mp-units V1 . If I got that wrong please correct me and I will apologise , so composability is not a problem here. But the thread is specifically about the issues arising from using effectively short global names to initialise a quantity which is a consequence of basically preventing the user from value initialising a quantity, unless supplying the units. For example the identifiers in |
Beta Was this translation helpful? Give feedback.
-
There is no need to apologize. We are not arguing or fighting here. We all want to find the best solution, and we need to discuss different approaches. Said that, I do not agree with that statement. How do you achieve
The usage of short global names has nothing in common with preventing the user from value initializing a quantity. Those are totally orthogonal. We would use the latter no matter of the way we spell the unit. The latter is about safety, and we believe it is the right way to go. Plenty of people (including the ISO C++ Committee experts) warned us about the safety issues that arise from the similar approach in
Yes, we will need to optimize compile times at some point. Until now, I have not spent any time on improving those as we still struggle to address the last missing pieces of the design, which have priority now. This library was designed from the very beginning with the C++ modules in mind, and support for C++ modules is arriving just now. Hopefully, soon, we will have some time and the possibility to work on the compile time performance.
This is why we describe this in one of the first chapters of your documentation: https://mpusz.github.io/mp-units/latest/getting_started/quick_start/#quantities. We may extend that admonition with more details if you think it is needed. |
Beta Was this translation helpful? Give feedback.
-
|
I've just extended the documentation here: https://mpusz.github.io/mp-units/latest/getting_started/quick_start/#quantities. |
Beta Was this translation helpful? Give feedback.
-
|
A side note on the compilation performance It is true that we can optimize the compilation performance, and we will do it for sure at some time in the future. However, usage examples compile so slow because of several additional reasons:
|
Beta Was this translation helpful? Give feedback.
-
It is a good example, but again with short unit names particularly it is not infallible #include <mp-units/systems/si/si.h>
#include <iostream>
#include <mp-units/format.h>
#include <mp-units/ostream.h>
#include <vector>
using namespace mp_units::si::unit_symbols;
namespace si = mp_units::si;
int main()
{
auto constexpr q1 = 1 * m;
auto constexpr m = q1 - q1;
std::vector< mp_units::quantity<si::metre> > v;
v.emplace_back( 42 * m );
std::cout << q1 / v[0] <<'\n';
return 0;
} |
Beta Was this translation helpful? Give feedback.
-
|
Sure! |
Beta Was this translation helpful? Give feedback.
-
|
We could not provide those at all and leave the definition of short names to the user. It would even improve compile times a lot as this is the biggest header file to process right now (https://github.com/mpusz/mp-units/blob/master/src/systems/include/mp-units/systems/si/unit_symbols.h). However, there are many places where they can be used and improve the usability and readability of code. This is why we decided to provide them with the library. |
Beta Was this translation helpful? Give feedback.
-
|
Compilation time is probably a discussion for another issue, so I should open that now |
Beta Was this translation helpful? Give feedback.
-
|
Sure, please do. Although it may take a while before we start tuning up the compile-time performance. |
Beta Was this translation helpful? Give feedback.
-
In the above example I used specifically an anonymous (unnamed) namespace , which emphasises that the names have internal linkage, so are only available in the particular translation unit, they are defined in. but the example here is enough for me to recommend avoiding this style as far as possible. Bringing the entire unit short name set into a large function is arguably as bad. You have the opposite problem. One day a new unit may be added to the namespace and quietly hide a value from an outer scope FWIW I don't know if this is correct behaviour in the following (gcc-13) , but if you uncomment the SOMETHING define assert fires, and not if you don't. And this is why I probably will recommend not to use any of these short unit aliases! #include <format>
#include <iomanip>
#include <iostream>
#include <mp-units/format.h>
#include <mp-units/ostream.h>
#include <mp-units/systems/si/si.h>
#include <mp-units/systems/international/international.h>
#include <cassert>
using namespace mp_units;
#define SOMETHING
void f(){
//...
#if defined SOMETHING
quantity km = { 1.e-3, si::metre};
#endif
//...
{
using namespace si::unit_symbols;
auto q2 = 10 * km;
assert( q2.unit == si::kilo<si::metre>);
}
}
int main()
{
f();
}
|
Beta Was this translation helpful? Give feedback.
-
|
FWIW in pqs a unit is composable using op * , op / , etc which returns a unit but there is no multiplication of a unit by a number. in using mp-units I would recommend always using the mp-units value constructor that keeps the number and the unit separate: auto q = quantity{number, unit};Interestingly, in #558 (comment) if I replace with I get an error message when SOMETHING is defined and that seems to be a big advance EDIT: In fact if I got around to reworking pqs, I think that {number, unit} is the value ctor I would opt for. It allows composability of math.numbers and of physical.units while keeping them in their separate domains, and the quantity constructor is a 'black box' which unites the domain of maths and physics in a quantity system |
Beta Was this translation helpful? Give feedback.
-
Sure, this is a correct behavior. If you define a local variable
Sure, if you want to do non-trivial logic in your function, it is perfectly fine not to use symbols and stick with full unit names. Also, you probably should not recommend using auto v = 10 * si::kilo<si::metre>;
auto q = v * si::kilo<si::metre>;
|
Beta Was this translation helpful? Give feedback.
-
|
We could consider an alternative approach here. Instead of having to do using namespace unit_symbols;
auto q = 42 * si::m;which would be much harder to hijack. Also, this would allow us to easier disambiguate between The only downside here would be a bit harder definition of symbols as we would need to exit the |
Beta Was this translation helpful? Give feedback.
-
I don't want to have to be clever and trip up my users and then tell them how dumb they are for not knowing the intricate rules at work here. I consider myself a reasonably competent C++ programmer and I didn't know the rules here. |
Beta Was this translation helpful? Give feedback.
-
Sure , units shouldn't be mixed with numbers. They are different domains. This is the best approach for clarity. auto speed_unit = mp_units::si::metre / mp_units::si::second;
mp_units::quantity q = {20.0f, speed_unit};
float value = q.numerical_value_in(q.unit);
std::cout << value << '\n';
auto area_unit = mp_units::si::metre * mp_units::si::metre;
mp_units::quantity q1 = {10, area_unit};
value = q1.numerical_value_in(q1.unit);
std::cout << value << '\n';
|
Beta Was this translation helpful? Give feedback.
-
|
I am considering adding support for additional construction helpers (the multiply syntax will remain). quantity q = delta<K>(300);
quantity_point qp = absolute<K>(300);
static_cast(q.in(deg_C) != qp.in(deg_C).quantity_from_zero());@kwikius, do you think it is a step in the right direction? |
Beta Was this translation helpful? Give feedback.
-
This probably is not going to happen because the constructor syntax does not compose. Just compare the two definitions: quantity q1 = quantity{7, non_si::hour} + quantity{20, non_si::minute} + quantity{35, si::second};
quantity q2 = 7 * h + 20 * min + 35 * s;The second one is so much easier to use. However, as it is clearly stated in our docs (https://mpusz.github.io/mp-units/latest/getting_started/quick_start/#quantities), the multiply syntax is not mandatory, and explicit constructor call may be used instead. The same applies to unit_symbols, which are not provided by default. Users have the choice to decide which style best suits their needs. |
Beta Was this translation helpful? Give feedback.
-
Yes. All the problems there are due to being able to multiply numbers ( and quantities) by units. Neither quantities nor number should be composable with units, I think the practise and syntax started with Walter Browns SI units library. https://lss.fnal.gov/archive/1998/conf/Conf-98-328.pdf . Anyway, there is an alternative value ctor provided so I will try using that exclusively and see how that goes. |
Beta Was this translation helpful? Give feedback.
-
|
I would actually be really interested in hearing some feedback from you when you are done. My understanding is that conversions from unsafe raw values to quantities should happen only in a few entry points to a program, and those should be limited to a very narrow scope to prevent potential safety issues. Mixing unsafe raw quantity values and strong quantities in the same context might be a recipe for disaster. |
Beta Was this translation helpful? Give feedback.
-
You are being somewhat unfair to the 2 arg ctor here. Short names for units would be much more acceptable if they didnt compose with numbers and quantities so what is good for the goose is good for the gander // may be a better way to do this
auto q = []( auto v, auto U) { return quantity {v, U };};
quantity q1 = q(7, h) + q(20, min) + q(35, s);
quantity q2 = 7 * h + 20 * min + 35 * s;
std::cout << " q1 = " << q1 <<'\n';
std::cout << " q2 = " << q2 <<'\n'; |
Beta Was this translation helpful? Give feedback.
-
You are going to have to write some quite dirty code, to move a legacy library from one to the other. Ardupilot is a great example of using floats for everything and you spend a lot of time checking units . ( Bear in mind ArduPilot is huge and I suspect there would be a lot of resistance to change) Here is one part that might be of interest to you . ( Ardupilot can do autonomous thermal soaring) In comparison to |
Beta Was this translation helpful? Give feedback.
-
|
The killer application for mp-units and ardupilot would be if use of units would increase speed and reduce code size. I am pretty sure that it can FWIW. There are piecemeal efforts to streamline units e.g ArduPilot/ardupilot#25768 . Ardupilot uses degrees, centidegrees and radians pretty much randomly throught out the codebase. |
Beta Was this translation helpful? Give feedback.
-
I am not sure what the ArduPilot code looks like. If it has some strong type wrappers (e.g., |
Beta Was this translation helpful? Give feedback.
-
Yes, I totally understand the issue, as I contributed for a few years to the http://lk8000.it. It was 15 years ago, and this is when I realized how important it is to solve the issue of units in similar code bases. I work on it from then... |
Beta Was this translation helpful? Give feedback.
-
Promote the use of |
Beta Was this translation helpful? Give feedback.
-
The use of names such as 'm' effectively in the global namespace doesn't scale well. I have been looking into trying to use mp-units in a real library and selected ArduPilot, since I know it well. I did a search for ' m ' in the codebase in .cpp and .h files and came up with around 350 hits.
mis of course only one of many very short names which are brought in by theusing namespace mp_units::si::unit_symbolsdeclaration.EDIT: for example :
The rough solution I came up with was a namespace alias, but is there another official alternative way to initialise quantities than these very unit names, because I can see them being very problematic if you are trying to update an existing codebase and this is the only way to initialise a quantity.
Below is a small selection of the results of the search
Beta Was this translation helpful? Give feedback.
All reactions