Documentation, overloaded operations, compilation issue

[Lecrapouille]

Is your feature request related to a problem? Please describe.

Hi sir ! Make a long time since https://github.com/bernedom/SI/issues/93 I still have not yet converted my float into SI in my personal project :( I'm currently testing your library back and at the same time testing this one https://github.com/nholthaus/units.

To help beginners starting with your lib, I think you could add some concrete some basic examples in your documentation. Here is an attempt of mine concerning tricycle kinematics. https://github.com/Lecrapouille/BacASable/tree/master/LawControl/SI

The code using your lib is here https://github.com/Lecrapouille/BacASable/blob/master/LawControl/SI/si.cpp and the code using units is here https://github.com/Lecrapouille/BacASable/blob/master/LawControl/SI/units.cpp (feel free to use this code).

I'm sorry, time is currently missing for me for writing this ticket (I do it anyway else I'll forget to write it), as well for making a C++ class of si.cpp like done in units.cpp, but currently si.cpp does note compile because (speed / wheelbase) fails compiling (I hope the issue does not come from me). In addition, I noticed some weak points compared to the other lib:

Operators that could be overloaded:

• The x += dt * ... does not compile, we have to write x = x + dt * ... instead.
• The << speed does not compile. We have to write << speed.value() instead (same for x, heading). In the other lib the unit is also displayed (like 2 mps).
• Less important but si::cos(heading) does not exist, we have to write instead std::cos(heading.value()) this just make the code more compact but let possible mistake like std::cos(x.value()) with x typed of metre_t.

Describe the solution you'd like

• Implement overloading operators such as +=, <<, cos() ...
• Add some basic examples using your lib.
• In your documentation, you are using auto but it is better to give the real type like SI::metre_t<double> because we have to find their type back when passing them as function arguments.

Describe alternatives you've considered Implemented in https://github.com/nholthaus/units.

Additional context n/a

[bernedom]

Hi again, thanks for the feedback.

• What compiler and version of SI are are you using to compile? With g++ 7 and newwer x += dt does not compile.

• If you include the header <SI/stream.h> you can output the units using streams. The following will print out 1 m/s

  
  #include <SI/stream.h>
  #include <SI/velocity.h>
  #include <iostream>

constexpr auto speed = 1_m / 1_s; std::cout << speed << std::endl;

* Adding the math functions like `si::cos` `si::sin` etc. is a good idea. Will add it in one of the next releases. 
* On the usage of `auto` I consider this a matter of code style and if you're using clangd as a language server in vscode you get a nice annotation of the underlying type. 

![image](https://user-images.githubusercontent.com/15666436/208318213-78255a65-7773-49da-9265-d783180c061a.png)

[Lecrapouille]

@bernedom Thanks

• I have updated my code!
• My bad for SI/stream.h the << works now :0)
• For auto nice hack with visual code but how what I dislike when defining member variables as auto is when passing them to functions, you have to give the real type but if auto === X<long double> in SI version 1 but in version 2 it becomes auto === X<long long double> since I used auto my code but keep using X<long double> in functions it will do implicit cast. So I did using Meter = SI::metre_t<long double>;
• I noticed another point: i as int and dt as seconds the i*dt does not work I had to do this (static_cast<long double>(i) * dt.value()) https://github.com/Lecrapouille/BacASable/blob/721a5707aa7646e4016188dd1bc743d4d1d88c6c/LawControl/SI/si.cpp#L126
• For += I'm using an up to date g++ version on Debian. Since I'm currently on holidays I cannot check the version but let say >= 10. I'm testing on Mac with clang and += works

  Apple clang version 13.0.0 (clang-1300.0.29.30)
  Target: x86_64-apple-darwin20.6.0
  Thread model: posix

• For the heading += dt * (speed / wheelbase) * u * std::tan(angle.value()); (or the = ... + ... form) https://github.com/Lecrapouille/BacASable/blob/721a5707aa7646e4016188dd1bc743d4d1d88c6c/LawControl/SI/si.cpp#L45 even with clang I have this error:

  g++ --std=c++17 -Wall -Wextra -ISI/include si.cpp -o prog
  si.cpp:35:30: error: invalid operands to binary expression ('SI::detail::unit_t<'v', std::ratio<1, 1>, long double, std::ratio<5, 18>>' and 'const SI::detail::unit_t<'L', std::ratio<1, 1>, long double>')
    heading += dt * (speed / wheelbase) * u * std::tan(angle.value());
                     ~~~~~ ^ ~~~~~~~~~
  SI/include/SI/detail/unit.h:491:1: note: candidate template ignored: requirement 'std::is_integral_v<SI::detail::unit_t<'v', std::ratio<1, 1>, long double, std::ratio<5, 18>>>' was not satisfied [with _type = SI::detail::unit_t<'v', std::ratio<1, 1>, long double, std::ratio<5, 18>>, _symbol = 'L', _exponent = std::ratio<1, 1>, _rhs_type = long double, _ratio = std::ratio<1, 1>]
  operator/(const _type &lhs,
  ^
  SI/include/SI/detail/unit.h:510:1: note: candidate template ignored: requirement 'std::is_floating_point_v<SI::detail::unit_t<'v', std::ratio<1, 1>, long double, std::ratio<5, 18>>>' was not satisfied [with _type = SI::detail::unit_t<'v', std::ratio<1, 1>, long double, std::ratio<5, 18>>, _symbol = 'L', _exponent = std::ratio<1, 1>, _rhs_type = long double, _ratio = std::ratio<1, 1>]
  operator/(const _type &lhs,
  ^
  SI/include/SI/velocity.h:43:1: note: candidate template ignored: could not match 76 against 118
  BUILD_UNIT_FROM_DIVISION(velocity_t, length_t, time_t)
  ^
  SI/include/SI/detail/operator_helpers.h:26:18: note: expanded from macro 'BUILD_UNIT_FROM_DIVISION'
  constexpr auto operator/(const DIVIDEND_UNIT_T<_type, _ratio_lhs> &lhs,      \
               ^
  SI/include/SI/velocity.h:43:1: note: candidate template ignored: could not match 76 against 118
  SI/include/SI/detail/operator_helpers.h:48:18: note: expanded from macro 'BUILD_UNIT_FROM_DIVISION'
  constexpr auto operator/(                                                    \
               ^
  SI/include/SI/detail/unit.h:295:20: note: candidate function not viable: no known conversion from 'const SI::detail::unit_t<'L', std::ratio<1, 1>, long double>' to 'const long double' for 1st argument
  constexpr unit_t operator/(const _type f) const { return unit_t{value_ / f}; }
                 ^
  SI/include/SI/detail/unit.h:302:18: note: candidate template ignored: could not match 118 against 76
  constexpr auto operator/(
               ^
  SI/include/SI/detail/unit.h:320:18: note: candidate template ignored: could not match 118 against 76
  constexpr auto operator/(
               ^
  SI/include/SI/detail/unit.h:337:3: note: candidate template ignored: could not match 118 against 76
  operator/(const unit_t<_symbol, _exponent, _rhs_type, _ratio> &rhs) {
  ^
  SI/include/SI/detail/unit.h:346:19: note: candidate template ignored: could not match 118 against 76
  constexpr _type operator/(
                ^
  1 error generated.

[bernedom]

The error is there because you're dividing velocity by distance which is not yet supported. The resulting unit would be Hertz or 1/s. I will add this.

[bernedom]

Sorry for the long wait I had a bit of trouble finding the time. I added angular frequency to this branch here: https://github.com/bernedom/SI/tree/feature/add-v-divided-by-L with this your original example should compile if you set the parentheses correctly. The example is in the file multi_unit_tests.cc

It would be cool if you could check if the calculation indeed works correctly, as I am not really sure there and lack the domain knowledge of your example to determine if the results make sense.