unitsofmeasurement / indriya

JSR 385 - Reference Implementation
Other
115 stars 40 forks source link

Support interoperability to other unit systems #402

Open jpink opened 10 months ago

jpink commented 10 months ago

My target is to create "custom" units for electricity prices: €/MWh and c/kWh. I added following classes:

public class CurrencyDimension implements Dimension, Serializable { // Copied from BaseUnit because its final ?!?
    // The symbol is '¤'
} 
public class Money extends AbstractNumberQuantity<Money> { // Copied from NumberQuantity because its final ?!?
    // Uses BigDecimal as a value.
    @Override
    public String toString() {
        // Workaround for tech.units.indriya.format.SimpleUnitFormat$DefaultFormat.format(SimpleUnitFormat.java:907)
        return getValue() + " " + getUnit();
    }
} 
public class CurrencyUnit extends AbstractUnit<Money> { // Again it would be nice to extend BaseUnit
    public final @NonNull Currency currency; // Internally uses java.util.Currency

    @Override
    public String toString() {
        // Workaround for tech.units.indriya.format.SimpleUnitFormat$DefaultFormat.format(SimpleUnitFormat.java:907)
        return getSymbol();
    }
}
public class CurrencyCentUnit extends CurrencyUnit { // Symbol is 'c' and name is currency display name + " cent".
    private final CurrencyUnit baseUnit;
}
public class Monetary extends AbstractSystemOfUnits { } // This seems to be optional

When i try to print quantity i get error:

java.lang.IllegalArgumentException: Cannot format given Object as a Unit
    at tech.units.indriya.format.SimpleUnitFormat$DefaultFormat.format(SimpleUnitFormat.java:907)

That is because my class isn't AlternateUnit or ProductUnit.

  1. Can you support standard Unit and Quantity interfaces?
  2. Can create another system of units and combine those different systems with this library? I think I should. Or should I fork this and do the modifications.
  3. And what benefits you get by marking all classes final?

I'm not planning to do currency exchanges (although its possible use some service). Just playing on same currency and combining to SI system.

jpink commented 10 months ago

I tried to create separate system using only javax.measure (not to extend tech.units.indriya.* classes). It is working correctly until I try to combine two systems by ProductUnit.ofQuotient:

EURO.toString() // Prints "€"
EURO.divide(Units.MEGAWATT_HOUR).toString() // Prints "null/MWh"

How can I register my Euro unit to SimpleUnitFormat.unitToName map?

I'm out of ideas. I try to use UoM in openHAB, but in this case it seems impossible to tell the unit for the system. Sadly I have to fall back use only BigDecimals.

jpink commented 9 months ago

Now I found the way to introduce my units using SimpleUnitFormat.getInstance().label(unit, unit.getSymbol())and prossibly extending classes may also work. Investigating...

keilw commented 9 months ago

Yes, that's indeed the way. Note, in the most recent Indriya versions, there is SimpleUnitFormat.getInstance() which returns a shared instance across the JVM / application lifetime, while SimpleUnitFormat.getNewInstance() creates a new instance that is local to a particular class or session, where label() is not so persistent. Allowing to adjust it for a particular purpose, e.g. a batch run, reporting or something similar, where you might want special settings that do not affect the rest of your application.

For EBNFUnitFormat it would be a little trickier, because you have to create a SymbolMap first, and then one could also use label() on that instance before creating an EBNFUnitFormat instance with it. I just created ticket #403 to make that easier.

jpink commented 9 months ago

I don't know what EBNF means :confused:

Now i faced minor issue. Let's say that USD and AUD have the same symbol $. Is there any way to embed some metadata (currency code) in the BaseUnit? The class is final, so I can't extend it. The idea what I got, is to create own dimension for each currency. UnitDimension.parse(char symbol) allows only one character and there is 180 currencies. So maybe I can find free space from Unicode. Or it is possible to extend UnitDimension, but if I understood correctly there shouldn't be multiple dimensions with same symbol? Or can I "force" currency code (three chars) as the dimension symbol?

keilw commented 7 months ago

I was thinking of a "hybrid" between this JSR and JSR 354 (Money and Currency) for quite a while, but it is a bit complicated. @dautelle had a money implementation back in the days of JSR 275.

BaseUnit should not be extended because it meets a very special purpose, the SI base units, nothing else. Not sure, why you would use Dimension for currencies, aren't those units? You might have to play a bit with the label() method of SimpleUnitFormat, but that way USD and AUD could have distinct names but the same label ("$")

dautelle commented 7 months ago

Hello, It is feasible (ref. Money) but currency exchange is not symmetrical (EUR => USD is not the same as USD => EUR), in other words how you convert is important (exchange rate is a tuple). Cheers, Jean-Marie

Le mer. 6 déc. 2023 à 20:18, Werner Keil @.***> a écrit :

I was thinking of a "hybrid" between this JSR and JSR 354 (Money and Currency) for quite a while, but it is a bit complicated. @dautelle https://github.com/dautelle had a money implementation back in the days of JSR 275.

BaseUnit should not be extended because it meets a very special purpose, the SI base units, nothing else. Not sure, why you would use Dimension for currencies, aren't those units? You might have to play a bit with the label() method of SimpleUnitFormat, but that way USD and AUD could have distinct names but the same label ("$")

— Reply to this email directly, view it on GitHub https://github.com/unitsofmeasurement/indriya/issues/402#issuecomment-1843544183, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPH2KVEZPNOHGCVZ72KXVDYIDAJTAVCNFSM6AAAAAA4WXIZEOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBTGU2DIMJYGM . You are receiving this because you were mentioned.Message ID: @.***>

J-N-K commented 6 months ago

I'm working on a similar problem (https://github.com/openhab/openhab-core/pull/3503). While conversion of currencies (we use symmetrical only, but the issue is similar) works fine, product units (like EUR/kWh) don't work.

The reason is that UnitCompositionHandlerYieldingNormalForm throws an NPE:

java.lang.NullPointerException: no normal-form order defined for class 'org.openhab.core.internal.library.unit.CurrencyConverter'
    at java.util.Objects.requireNonNull(Objects.java:336) ~[?:?]
    at tech.units.indriya.internal.function.simplify.UnitCompositionHandlerYieldingNormalForm.isNormalFormOrderWhenCommutative(UnitCompositionHandlerYieldingNormalForm.java:112) ~[?:?]
    at tech.units.indriya.internal.function.simplify.UnitCompositionHandlerYieldingNormalForm.compose(UnitCompositionHandlerYieldingNormalForm.java:80) ~[?:?]
    at tech.units.indriya.function.AbstractConverter.concatenate(AbstractConverter.java:179) ~[?:?]
    at tech.units.indriya.unit.ProductUnit.getSystemConverter(ProductUnit.java:330) ~[?:?]
    at tech.units.indriya.AbstractUnit.getConverterToAny(AbstractUnit.java:327) ~[?:?]

because it has a fixed set of supported converters and it is not possible to add new converters. The set of converters is retrieved from Calculus.getNormalFormOrder() and initializes and returns Calculus.normalFormOrder which is inaccessible.

For us using a DoubleMultiplyConverter would be fine, but that's also not possible, because in our new class CurrencyUnit we can't instantiate that because all constructors are package private.

keilw commented 6 months ago

Thanks for mentioning this. I suppose, you do have some sort of CurrencyConverter or similar subclass of AbstractConverter?

The Calculus.normalFormOrder is used in a very isolated way, so @andi-huber do you see any problems adding a method like registerUnitConverter, somewhat similar to setCurrentNumberSystem? And if added, how should it handle the order of new, custom converters? Before or after 99 for AbstractConverter.Pair?

J-N-K commented 6 months ago

Correct, it's a sub-class of AbstractConverter. I believe the proposed method would also allow for asymmetric converters (if they inherit from AbstractConverter) and thus be useful for @jpink.

What I'm doing now is adding it via reflection (with order 1000), but that is of course a dirty hack.

jpink commented 6 months ago

BaseUnit should not be extended because it meets a very special purpose, the SI base units, nothing else. Not sure, why you would use Dimension for currencies, aren't those units? You might have to play a bit with the label() method of SimpleUnitFormat, but that way USD and AUD could have distinct names but the same label ("$")

@keilw: I added currency "¤" dimension, because I don't understand the dimension system and don't want to mess with existing ones. $ symbol isn't problem for me for now. I use ISO code for parsing using alias. And currencies are shown using symbols. The user knows the context.

@dautelle: I don't need to exchange currencies. I only want to convert units eg. 100 €/MWh => 10 snt/kWh. That's easy divide by ten without the units library, but I have fetish to use it 😄

For us using a DoubleMultiplyConverter would be fine, but that's also not possible, because in our new class CurrencyUnit we can't instantiate that because all constructors are package private.

@keilw, @J-N-K: I don't understand what are the benefits of closing every single class in the implementation if they aren't the public API. Developers can always fork codes and patch them (like I often do). But that causes that I can't integrate my implementation to OpenHAB. Currently my UoM works as I like inside the binding, but OpenHAB doesn't understand those units. So I use full Quantities inside of the binding and tell only numbers (without units) to OpenHAB.

BaseUnit should not be extended because it meets a very special purpose, the SI base units, nothing else.

@keilw: I really understand if there is good reasons to close something. But then you should document those (in the code) and tell what is the correct way to do things or provide some workaround (in documentation).

Sorry but I got a little pissed of this library. I didn't got it working in three weeks, so I give up, let it be and done thing the other way.

J-N-K commented 6 months ago

I would like to ensure that one thing is 100% clear: The statement of @jpink is in no way endorsed by openHAB maintainers group and does not reflect my or any other opinion I know of.

keilw commented 6 months ago

Thanks for the clarification. The beauty of open standards is, there are multiple implementations, if @jpink prefers to write his own, or use another one, sure no problem. Will use this ticket to see, how Calculus could be extended for additional converters.

andi-huber commented 6 months ago

In a broader sense having units for money with different currencies (USD, EUR, ...) falls in the same ballpark as having units like RADIAN and DEGREE. All those units have same dimensions (dimensionless in that case) but cannot necessarily be converted into one another. (I believe you cannot convert RADIAN into USD, or RADIAN int STERADIAN.) We say those units are not commensurable. However having categories of units that form a set of commensurable units, would be nice. E.g. having a set of units for the category money that allows for converters to be defined within this category.

Not sure if the current API can do this or if it can be extended to do this.

I don't have immediate answers. Perhaps I find time later this month.

Some related material:

  1. There can be only one ONE https://github.com/unitsofmeasurement/indriya/issues/361
  2. STERADIAN/RADIAN discussion https://github.com/unitsofmeasurement/indriya/issues/257
  3. Introduction of Unit Categories https://github.com/unitsofmeasurement/indriya/pull/258
  4. Unit Equivalence (Wiki) https://github.com/unitsofmeasurement/indriya/wiki/Unit-Equivalence
keilw commented 6 months ago

The use case of OpenHAB is mainly something like EUR/kWh, or maybe the cost per ton of CO2, etc., not converting Euros into Dollars or Radian. In the meantime, what about adding new custom converters (which as mentioned above are needed for EUR/kWh or similar) to normalFormOrder in Calculus? And how should its order look like, between the built-in ones and AbstractConverter.Pair or at the end of the list?

jpink commented 5 months ago

@andi-huber: I agree that there will be challenges to convert many different kinds of dimensionless units. But I think this library wasn't birth because of ideological or theoretical reasons. It should solve real world problems and there is no need to try convert radians to USD.