Clarify behavior of Quantity arithmetic on shifted units

[desruisseaux]

Clarify the behavior of Q₁ + Q₂ or Q * n (with n a unitless number) when the unit of measurement of Q, Q₁ or Q₂ is "shifted". Example of shifted units are:

• Temperature in Celsius, with °C defined as K - 273.15 K.
• Density sigma-t, with σT defined as ρ - 1000 kg/m³ (used in oceanography).
• Other shifted units in other scientific domains?

Taking temperature in °C as an example, addition can happen in at least two different contexts:

• T₁ + T₂ : there is some legitimate uses for such additions, but they are rare.
• T + ΔT : a more common case.

In the T₁ + T₂ case, both quantities must be converted to K before addition. So 20°C + 10°C = 303.15°C because T₂ = 283.15 K. In the T + ΔT case, 10°C is an increment (ΔT). So in this later case 20°C + 10°C = 30°C.

In current API, there is no way to know if a number is an absolute quantity or an increment. So there is no way to know if we are in the T₁ + T₂ case or in the T + ΔT case. If we want to differentiate those quantities, we may need to:

• Add a new method in UnitConverter: deltaConvert(double), which convert the given value without applying the offset. This is similar to deltaTransform in java.awt.geom.AffineTransform.
• Introduce a new interface: Increment<Q extends Quantity<Q>>.
• Defines arithmetic rules as below: in any arithmetic operation like M₁ + M₂ or M * n, implementation SHALL do the following steps at least conceptually (implementations are allowed to avoid conversions in some cases provided that the numerical result is the same):
  • All M values SHALL be converted to system units before addition or multiplication.
    • Conversions of Quantity instances shall be done with UnitConverter.convert(…).
    • Conversions of Increment instances shall be done with UnitConverter.deltaConvert(…).
  • Result type is defined by the following table:
    • Quantity + Quantity = Quantity
    • Quantity + Increment = Quantity
    • Increment + Quantity = Quantity
    • Increment + Increment = Increment
    • Quantity * n = Quantity
    • Increment * n = Increment
  • Result value is converted back to the unit of measurement of the first operand (M₁).
    • Using UnitConverter.convert(…) if the result type is Quantity.
    • Using UnitConverter.deltaConvert(…) if the result type is Increment.

Requires #98
Requires #99

[desruisseaux]

Note: see https://rechneronline.de/temperatur/temperature.php for example of operations on temperatures. Even if the above proposal is not apply, at least we should add tests making sure that 20°C + 10°C = 303.15°C, not 30°C, unless we clearly state in the javadoc a policy about when quantities are considered increments.

[keilw]

This sounds rather intrusive for adding it to the API and introduce something like javax.measure.Increment or add a new method to the UnitConverter interface in that same package. Especially if it does not apply to at least 90% of all quantities (in javax.measure.quantity plus e.g. si-quantity or systems-quantity)
The mentioned other example Density is part of si-quantity but it does not seem used by any of the unit systems yet.
We decided to keep Range or Measurement in implementation modules like Indriya. Or ComparableQuantity which in theory (Java SE8 and above now) could even be merged into Quantity, but I'm not sure about that either (will ask this in a separate ticket)
So while using Comparable, Serializable or Prefix apply to pretty much every quantity (maybe the only ones where Prefix may not make much sense are Temperature and a few others)
So a gut feeling tells me, the small amount of quantities (out of at least 60 in the 3 quantity modules/packages as of now, how many are confirmed to need Increment?) does not justify an extra API element, but it seems perfectly fine to define it similar to Range or ComparableQuantity.

Beside UnitConverter.convert() does not return a Quantity, so adding a method that does would break with its current purpose. If AddConverter (used between CELSIUS and KELVIN right now) does not work for this case, why not introduce another UnitConverter implementation that does? If a Quantity or similar element like Increment has to be applied and returned, the QuantityFunctions class looks like a good place for methods like delta() etc.

[desruisseaux]

I never proposed to have a method returning Quantity in UnitConverter. I proposed a deltaConvert(double) method, returning a double like the existing convert(double).

The other example is not Density, but sigma-t. Density has nothing special compared to other units; it sigma-t which is a shifted unit.

Yes, addition of an Increment interface is intrusive. But conceptually it applies to all units; not only temperature and sigma-t. It is not apparent for other units because in their case, Quantity and Increment have the same behavior.

The situation is not comparable to Range. Range can be defined in a totally independent way; supporting ranges in an external API do not require any change in JSR-363 API. But the situation is not the same for Increment. If arithmetic operations are defined in Quantity, then they need to define precisely their behavior with shifted units - current API is not satisfying. We can either talk about increment in the javadoc, or introduce an explicit Increment type.

[keilw]

The wording sounded a bit like the interface would be expected as a return value. Nevertheless if there were two methods there it had a rather significant impact on the Spec and other places to clarify how to use them and when. It is a bit like the new fireAsync() methods introduced with CDI 2 that are also applied and used in parallel to those already existing. The overhead in the API would not be that big, but Spec, RI and TCK would have to be changed significantly. Not entirely sure, if it had to be on the top level, but if you envision a Quantity.add(Increment) method then it must be, see Prefix and its relation to Unit.

Similar to CDI 2 I would probably call such new method UnitConverter.convertDelta() or UnitConverter.convertIncrement() rather than deltaConvert().

While Range could also be done using e.g. Google Guava, the Measurement interface is something e.g. OSGi Measurement has defined in its core API.

[keilw]

Wonder, if someone from the SmartHome team like @htreu or @kaikreuzer might help with that at least as Contributors?

[desruisseaux]

I'm find with convertDelta or convertIncrement method names.

Introducing an explicit Increment interface is one possibility. Another possibility is to not introduce new interface an introduce the following methods in Quantity instead:

• addIncrement(Quantity)
• subtractIncrement(Quantity)
• multiplyIncrement(Quantity)
• divideIncrement(Quantity)

[keilw]

While it adds extra methods that may actually be less intrusive and complex. #96 asked about possibly adding a few methods now in ComparableQuantity into the API. If the result of these 4 candidates is always a Quantity, would method overloading with add(Quantity, OperationType) also work? OperationType would simply be an enum either standalone or a static inner type in Quantity.

Let's do a quick vote.
Please state your preference for the result and argument of "increment" operations:

[keilw]

A) Add a new Increment type to the API.

[keilw]

B) Reuse Quantity and add new methods like addIncrement(Quantity) or add(Quantity, OperationType.INCREMENT) to it.

[keilw]

I hope others will also cast their vote. I voted B also because the whole formatting topic for quantities would get even more complicated if we had to differentiate between Quantity and Increment.

[desruisseaux]

Before to vote, is the problem clear to everyone? The mathematical problem (regardless any programmatic API), whether Increment would be sufficient of if there is a risk of need for more types, the pros and cons of different proposal?

For example is it clearly understood that while multiplyIncrement(Quantity) may seems less intrusive than multiply(Increment) + the addition of an Increment interface, it is also less convenient for the users because (s)he has to remember herself/himself if the last operation (s)he did resulted in an increment rather than a quantity?

C) Leave all types and method signatures as today, and add the following method in Quantity interface instead:

• LevelOfMeasurement getLevel();

Where LevelOfMeasurement is an enumeration containing at least INTERVAL_SCALE and RATIO_SCALE (see https://en.wikipedia.org/wiki/Level_of_measurement#Interval_scale). Then the behavior of Quantity.add, subtract, multiply and divide depends on the LevelOfMeasurement of each quantity.

[keilw]

I know it's a little messy now, but I could update your comment and created an option C there.
Which I find even less intrusive. Let's try to call it MeasurementLevel, regardless of being an inner enum or standalone type, to better blend in with existing types like MeasurementException.

[filipvanlaenen]

Note: an example of a legitimate use for T₁ + T₂ would be the calculation of the average over a set of temperatures (e.g. (T₁ + T₂)/2), but in that case, the offset is eliminated again by the division.

[keilw]

Thanks, guess that speaks for sticking to Quantity. Any preferences of A, B or C @filipvanlaenen ?

[filipvanlaenen]

Actually, I'd rather opt for
D) Don't make a change (at least not in the API)

20°C + 10°C = 303.15°C sounds very counter-intuitive. I'm pretty sure this is going to produce a lot of bug reports.

Furthermore, I remember from thermodynamics back at university (a quarter of a century ago…) that temperature calculations were done in Kelvin, and nothing else. Not sure for physics, but I seem to remember that there too, you did the conversion into Kelvin when dealing with absolute temperatures.

I wonder whether we really have a problem for density sigma-t too, but the oceanographer would have to answer that.

Complicating factor: for integer quantity types, 20°C + 10°C will be 303°C, right?

[keilw]

I morphed your comment into option D. I take @filipvanlaenen D was your choice then?;-)

@unitsofmeasurement/experts and @unitsofmeasurement/contributors please consider voting on the three options, we have two thumbs up on the initial text, but that is not clear whether you prefer Option A, B or C to accomplish it.

[keilw]

@desruisseaux Would it be possible to vote on a choice (A-D) as well, since you presented 3 of them?
@otaviojava, @htreu could you place your thumbs-up on one of the solution paths as well? I take it you don't prefer option D (don't do it at all) but we have to chose which of the options to go for if we want to do anything at all on this.

Thanks,
Werner

[desruisseaux]

Yes, calculation in thermodynamic must be done in Kelvin. This is why 20°C + 10°C = 303.15°C if neither 20°C or 10°C are interval. Maybe 20°C + 10°C = 576.3 K would be less confusing.

Option D (no API change) would require that we choose a behavior for Quantity.add(Quantity) (e.g. is the argument considered an absolute measurement or an increment?) and we document it in the Javadoc. Users would need to think about whether that behavior fits his need.

I would go for a A amended by C. I.e. do everything described in A, except that the Increment type is replaced by a LevelOfMeasurement getLevel() method (maybe different name) in Quantity. Advantages:

• Only one method to add instead than 4.
• If in the future we discover that there is another level of measurement than we need to take in account, it would be only one enumeration value to add in LevelOfMeasurement instead than 8 new methods in Quantity.
• Avoid the need to know the level of measurement at compile-time (unless Increment is defined as a subtype of Quantity, which would be a new alternative E). Since the quantity or increment may be the result of another calculation, we may not know at compile time if the argument given to a Quantity.add(…) method is an Increment or a Quantity.

[desruisseaux]

Isn't too early for vote? I'm not sure that we have well explored the proposed plan and alternatives. Each comment is raising new alternatives.

[keilw]

Well we can't have too many options. I was initially worried, it could better be dealt with in a custom or implementation-specific way. And @filipvanlaenen (or what he proposed as D) seems to share that concern.
We should find out a trend (and of course like the "Compound" question those who are in favor of a certain approach also need to provide support or at least enough input on it)

[kaikreuzer]

In general, I would vote for D, i.e. do not handle increments at all.
But what I then expect is a consistent behavior, but I think for this, something needs to be changed in the implementation. What I found in eclipse-archived/smarthome#5697 is that 0 °C + 0 °C = 0 °C while 32°F + 0 °C = 64 °F (=17.8 °C), which is simply unexpected from whatever perspective you look at it - thus I'd consider it a bug as you get different results depending on the unit that you used for the input.

20°C + 10°C = 303.15°C sounds very counter-intuitive. I'm pretty sure this is going to produce a lot of bug reports.

@filipvanlaenen If we do not introduce any increment handling, this would be the correct result, wouldn't it? So if you do not like that, why do you suggest to go for D?

[dautelle]

This is an interesting problem which is the same as for dates (often represented by a duration since January 1st, 1970). You don't add dates together (does not make senses) but you can add a duration to it in order to get a new date:
Duration Since Jan 1st 1970 + Any Duration = Duration (implicit from Jan 1st 1970)

For our temperatures we have:
Temperature above absolute Zero + Any Temperature = Temperature (implicit above absolute zero).

In other words:
10°C + 10° K = 20 °C
but 10°C + 10° C = 10°C + 283 °K = 293° C

My choice is therefore D (nothing to do since I believe that it is the default behavior).
You can understand this as quantities being always "delta/differences", in some cases a delta from an absolute predefined point.

[kaikreuzer]

since I believe that it is the default behavior).

False assumption.
Check this unit test, which currently passes. It shows that 1 °C + 2 °C = 3 °C (and not 276 K as you claim). That's what I call the bug in the current implementation. It converts the second value always to the unit of the first one (instead of converting it to K) and then does the calculation. This results in the behavior that you get different results depending on the unit of your first value, which is simply wrong.

[kaikreuzer]

Just for the full picture: This unit test actually fails, while I think that we all agree that 1 °C + 1 K should result in 2 °C in ANY case (treating the values as absolute Kelvin values being added as well as treating the second value as an increment).

Failed tests: 
  NumberExtensionsTest.operatorPlus_Quantity_Quantity_withDifferentUnit:68 
Expected: is <2 ℃>
     but: was <-271.15 ℃>

[andi-huber]

When stating 'is do-able without API change' yesterday, what I meant was, that from the code perspective, no new interfaces or methods would need to be introduced. But I agree, that the behavior and hence associated Java-doc would have to change, which strictly speaking one could also see as a change in API, yes.

[desruisseaux]

Adding the unit to convert to and store as could be one option

No, the problem is not that we need an extra option for the unit to convert. The target unit (not to be confused with the computational unit) does not matter, as long as the quantities are equivalent as defined in the wiki page.

The problem is to know if a quantity is a measurement or a difference (or increment). This is orthogonal with unit. It could be a sub-type of Quantity, or a property in Quantity in complement with the Unit. A property directly in Unit has also been suggested.

[keilw]

That with e.g. some enum would be the least invasive and most definition compliant (i.E. https://en.wikipedia.org/wiki/Level_of_measurement, we quote Wikipedia in other places, too) addition to the API.

[desruisseaux]

To summarize where we are now: this issue was about clarifying arithmetic operations on Quantity. I think we have reached an agreement that the only arithmetically consistent way before introducing new types/properties/methods is to require the rule stated above, despite its counter-intuitive consequences. Then I propose the following actions:

1. Update Quantity Javadoc with:
   • definition of quantity equivalence
   • above rule for arithmetic operations
2. Update implementations accordingly.
3. Close this issue if peoples agree with the javadoc.
4. Create a new issue for new types, properties or methods to add to the API.

[filipvanlaenen]

I don't think this is such a good idea. I would leave things like they are for now, and make the change in one API change instead of two. Also, I'm not so fond of API changes that go completely under the radar, i.e. a change in behaviour without a warning. There may be users out there for which the current implementation works just fine, and then suddenly, they're getting results that don't make much sense, without a compilation error or a runtime exception. As a user, I wouldn't be particularly happy with that.

[desruisseaux]

In my understanding, the API change - at least for the part standardized by JSR - would have to be done all in one shot, since JSR-385 is about "Unit API 2.0" and must be voted by JCP (Oracle and other members) before to be released.

However implementations may have as many releases as they want. So it is possible for Indriya to take a progressive approach if they want, but all Indriya official releases have to be on JSR-363 API as long as JSR-385 has not been officially approved (but Indriya may have development branches on JSR-385 API for testing purpose).

JSR-363 API said nothing about the behavior of Quantity arithmetic operations. So a change of behavior of Indriya in this aspect would not be a compatibility break with JSR-363. It may be a compatibility break with previous Indriya versions, but not with the specification. So I think it is up to Indriya project to manage this migration as they wish; I don't think it would interfere with JSR-385.

[keilw]

I think it's a good idea to create detail tasks where agreed on. Indriya did not exist until the new JSR was started. And Indriya 1.0 is a fully compatible variation of the Java 8 implementation that was not the RI earlier. It is a bridge to the new RI, actually 1.0 is still using the old "tec." namespace, so after Indriya 1.2 there are package changes, but the general structure is compatible. The only things that are going to change are e.g. MetricPrefix moving to the API and a few concrete classes like the PiConverter ones. Although the TCK and JCP EC likely won't care about such details, users and projects or commercial products (OpenHAB/SmartHome is the foundation for commercial SmartHome products like "Magenta SmartHome" by Deutsche Telekom) do care about predictability, so suddenly returning only SI units where a different unit passed as arguments to an operation was returned before is an absolute no-go. If conversion happens back and forth under the hood, that's a different story, but the overall behavior should not be broken across the RI compared to the current uom-se implementation. The old RI did not use BigDecimal, so it had known precision issues in some cases. That of course is an improvement by the new RI, which is why compatibility with uom-se is more important than the old units-ri.

[keilw]

Closing this after the creation of #130 and unitsofmeasurement/indriya#128

[keilw]

@kaikreuzer @htreu it took a while to apply that, but we just finished #130 and thanks to #131 there is a way to calculate e.g. temperatures (with an INTERVAL level of measurement) differently than other units.