Comparison without narrowing is inconsistent

[krevelen]

When type narrowing is disabled in the NumberSystem, inconsistency between comparison of A <> B and B <> A arise due to rounding errors. For example:

    Calculus.setCurrentNumberSystem(new DefaultNumberSystem() {
        public Number narrow(Number number) {
            return number; // disable narrowing
        }
    });
    final ComparableQuantity<Time> minute = Quantities.getQuantity(BigDecimal.ONE, Units.MINUTE);
    final ComparableQuantity<Time> seconds = Quantities.getQuantity(BigDecimal.valueOf(60), Units.SECOND);
    System.out.println("60 s = 1 min: " + seconds.compareTo(minute)); // 60 s = 1 min: 0 [correct]
    System.out.println("1 min = 60 s: " + minute.compareTo(seconds)); // 1 min = 60 s: -1 [incorrect]

As the rounding errors will always occur when inverting the unit-converter (see AbstractUnit.internalGetConverterTo(Unit)), perhaps a prior check should be added whether the comparison should be inverted instead, by comparing in reverse order if the latter unit is a multiple of the former, and then reversing the result, thus avoiding rounding errors. For example (in AbstractQuantity.compareTo(Quantity<Q>)):

public int compareTo(Quantity<Q> that) {
    if (this.getUnit().equals(that.getUnit())) {
        return numberSystem().compare(this.getValue(), that.getValue());
    }
    // work-around: compare smallest unit first, so larger unit is multiplied (which is more exact)
    final Unit<Q> thisSystemUnit = this.getUnit().getSystemUnit();
    final Unit<Q> thatSystemUnit = that.getUnit().getSystemUnit();
    if(thisSystemUnit.equals(thatSystemUnit)) {
        final double thisSystemUnitFactor = this.getUnit().getConverterTo(thisSystemUnit).convert(1.0);
        final double thatSystemUnitFactor = that.getUnit().getConverterTo(thatSystemUnit).convert(1.0);
        if(thisSystemUnitFactor > thatSystemUnitFactor) {
            return -1 * numberSystem().compare(that.getValue(), this.to(that.getUnit()).getValue());
        }
    }
    // not same base-unit, proceed normally
    return numberSystem().compare(this.getValue(), that.to(this.getUnit()).getValue());
}

[keilw]

@krevelen @andi-huber Is it disabled by default?

[andi-huber]

Generally speaking, there is no guarantee that comparing quantities gives consistent results. This comparison can only be answered within margins of numerical error, but there is no means (yet) for consumers of the library, to specify any margins of error. Also, I don't really have a good idea how to best do this. If in doubt, I'd rather have this operator removed, than leading users into potential traps.

(Number narrowing is not disabled by default. However, the issue with those quantity comparison operators is unrelated.)

[keilw]

Thanks @andi-huber I assigned you to this. If there was anyting to act on, feel free to do, otherwise also feel free to close it with the above or further comments.

[andi-huber]

Hi @keilw - the task, as I see it, would be to remove AbstractQuantity.compareTo(Quantity<Q>)).

[keilw]

@andi-huber If that helps, why not, but does it put a burden on other concrete types like NumberQuantity?

[krevelen]

Alternatively you could re-introduce a type similar to the Amount introduced by JScience for the (rejected) JSR-275 available at https://mvnrepository.com/artifact/org.jscience/jscience, with its Amount.approximates(Amount) method. There, bookkeeping of numerical error-ranges occurred within the Amount measurement. I'm guessing it would be buch easier "to burden concrete types" as @keilw warns, and provide an extended API as @andi-huber notes, e.g. NumberQuantity.approximates(Quantity, Number) and NumberQuantity.compareTo(Quantity, Number), where each second argument specifies the relative numerical error range. For now, I'll stick to my inverse/reverse-trick which multiplies to smaller unit values (rather than divide to larger unit values) so as to obtain an exact match/comparison.

[keilw]

@krevelen Thanks for the suggestion. I won't think we should add any extra layers on top of Quantity or AbstractQuantity beside what's already there, but I guess what was in Amount back then could in a somewhat similar form also be added to ComparableQuantity. Both possible methods if more than one was required. We already have a couple of boolean methods like isGreaterThanOrEqualTo(), so maybe isApproximate() might blend in with those a little better, but first let's discuss and plan the actual demand for such methods before tweaking their names. Could you create a more concrete feature request here please?

[keilw]

In the absense of a more detailed feature request, let's close this for now.