Units are order locked in multiplication or division

[vickoza]

No description provided.

[chiphogg]

Tell us some more! 😅

• What did you observe?
• What did you expect to observe?
• What problems is this causing you?

[mpusz]

Hi @vickoza! As @chiphogg already wrote, could you please provide more information about the issue? In order to resolve this issue and help you we need to understand the problem first.

[mpusz]

Duplicate of #624

[vickoza]

Sorry for the update the issue might be related but is not duplicated. the code was not my own but a modified example that show the issue

This code compiles

#include <mp-units/compat_macros.h>
#include <mp-units/ostream.h>
#include <mp-units/systems/international.h>
#include <mp-units/systems/isq.h>
#include <mp-units/systems/si.h>


#include <iomanip>
#include <ostream>

using namespace mp_units;
using namespace mp_units::si;
using namespace mp_units::si::unit_symbols; // mp_units::si::unit_symbols::
using namespace mp_units::international::unit_symbols;

template <Reference auto Unit, class T = double>
void thousands_scaled(std::ostream &out, mp_units::quantity<Unit, T> const &num) {
    cout << std::setprecision(fcc_iaru_precision)
         << std::setw(18);
    if(num < 0.00'000'000'000'1 * Unit) {
        quantity<si::femto<Unit>, double> n{ num };
        out << n;
    } else if(num < 0.00'000'000'1 * Unit) {
        quantity<si::pico<Unit>, double> n{ num };
        out << n;
    } else if(num < 0.00'000'1 * Unit) {
        quantity<si::nano<Unit>, double> n{ num };
        out << n;
    } else if(num < 0.00'1 * Unit) {
        quantity<si::micro<Unit>, double> n{ num };
        out << n;
    } else if(num < 0.01 * Unit) {
        quantity<si::milli<Unit>, double> n{ num };
        out << n;
    } else if(num < 0.1 * Unit) {
        quantity<si::centi<Unit>, double> n{ num };
        out << n;
    } else if(num < 1'000.0 * Unit) { 
        out << num;
    } else if(num < 1'000'000 * Unit) {
        quantity<si::kilo<Unit>, double> n{ num };
        out << n;
    } else if(num < 1'000'000'000 * Unit) {
        quantity<si::mega<Unit>, double> n{ num };
        out << n;
    } else if(num < 1'000'000'000'000 * Unit) {
        quantity<si::giga<Unit>, double> n{ num };
        out << n;
    } else if(num < 1'000'000'000'000'000 * Unit) {
        quantity<si::tera<Unit>, double> n{ num };
        out << n;
    } else if(num < 1'000'000'000'000'000'000 * Unit) {
        quantity<si::peta<Unit>, double> n{ num };
        out << n;
    } else {
        out << num;
    }
}

This code does not work because it does not compile

#include <mp-units/compat_macros.h>
#include <mp-units/ostream.h>
#include <mp-units/systems/international.h>
#include <mp-units/systems/isq.h>
#include <mp-units/systems/si.h>


#include <iomanip>
#include <ostream>

using namespace mp_units;
using namespace mp_units::si;
using namespace mp_units::si::unit_symbols; // mp_units::si::unit_symbols::
using namespace mp_units::international::unit_symbols;

template <Reference auto Unit, class T = double>
void thousands_scaled(std::ostream &out, mp_units::quantity<Unit, T> const &num) {
    cout << std::setprecision(fcc_iaru_precision)
         << std::setw(18);
    if(num < Unit / 1'000'000'000'000) {
        quantity<si::femto<Unit>, double> n{ num };
        out << n;
    } else if(num < Unit / 1'000'000'000) {
        quantity<si::pico<Unit>, double> n{ num };
        out << n;
    } else if(num < Unit / 1'000'000) {
        quantity<si::nano<Unit>, double> n{ num };
        out << n;
    } else if(num < Unit / 1,000) {
        quantity<si::micro<Unit>, double> n{ num };
        out << n;
    } else if(num < Unit / 100) {
        quantity<si::milli<Unit>, double> n{ num };
        out << n;
    } else if(num < Unit / 10) {
        quantity<si::centi<Unit>, double> n{ num };
        out << n;
    } else if(num < Unit * 1'000.0) { 
        out << num;
    } else if(num < Unit * 1'000'000) {
        quantity<si::kilo<Unit>, double> n{ num };
        out << n;
    } else if(num < Unit * 1'000'000'000) {
        quantity<si::mega<Unit>, double> n{ num };
        out << n;
    } else if(num < Unit * 1'000'000'000'000) {
        quantity<si::giga<Unit>, double> n{ num };
        out << n;
    } else if(num < Unit * 1'000'000'000'000'000) {
        quantity<si::tera<Unit>, double> n{ num };
        out << n;
    } else if(num < Unit * 1'000'000'000'000'000'000 ) {
        quantity<si::peta<Unit>, double> n{ num };
        out << n;
    } else {
        out << num;
    }
}

Is appears multiplication by a scaler is only valid one way it also does not allow division by a scaler value i this the operator should be overloaded for other multiplication and division of scaler values

[chiphogg]

This seems like a reasonable feature. Au already explicitly allows this, for example: https://godbolt.org/z/aT8WvsEqf

[chiphogg]

...That said, there's one important caveat. In Au, you would not be able to divide by an integer type. After all, integer division would truncate the result to 0, which is surely not what is desired. Au has a safety check to prevent this. You would need to, e.g., divide by 1'000'000'000'000.0.

But this is still better than multiplying by 0.00'000'000'001, because that real number can't be represented exactly in floating point.

[mpusz]

Should we support both syntaxes?

quantity q1 = 1 * m;
quantity q2 = m * 1;

Some time ago, we decided not to allow the q2 as this might result from an error in a code and make the code harder to reason about. Should we revisit this?

[mpusz]

The code below should work fine today:

if(num < 1 * Unit / 1'000'000'000'000) {

[chiphogg]

The code below should work fine today:

if(num < 1 * Unit / 1'000'000'000'000) {

Unfortunately, I think you're right! 😉 https://godbolt.org/z/no7aeG98o

[chiphogg]

Should we support both syntaxes?

quantity q1 = 1 * m;
quantity q2 = m * 1;

Some time ago, we decided not to allow the q2 as this might result from an error in a code and make the code harder to reason about. Should we revisit this?

I vaguely remember this discussion. And yet, I also explicitly allowed this syntax in Au. So now I'm wondering: why exactly did we disallow this? Maybe we had a good reason, and I should revisit my decision.

If we could dig up the examples we used in that discussion, that'd be helpful in deciding which way we should go.

[chiphogg]

The code below should work fine today:

if(num < 1 * Unit / 1'000'000'000'000) {

Unfortunately, I think you're right! 😉 https://godbolt.org/z/no7aeG98o

Actually... Au would have exactly this same problem, if we're dividing an integer-rep quantity by an integer: https://godbolt.org/z/ssv8W47M6 😳

Looks like we have the safety checks for dividing the symbol by the integer, because we know there's an implicit value of 1, and we'll always get truncated to 0. But doing integer division with integer-rep quantities is a common and legitimate use case, and I don't think there is anything special that either mp-units or Au should do to prevent this.