unitsofmeasurement / indriya

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

Conversion of Double#NaN with UnitConverter #287

Closed fawidmer closed 3 years ago

fawidmer commented 4 years ago

The following piece of code leads to a java.lang.NumberFormatException, as the Double.NaN value is attempted to be represented by a BigDecimal using BigDecimal#valueOf(double).

The expected behavior in my eyes would be to return Double.NaN.

UnitConverter converter = Units.GRAM.getConverterTo(Units.KILOGRAM);
converter.convert(Double.NaN);

Needs #254

DrDaleks commented 4 years ago

would that be related to https://github.com/unitsofmeasurement/indriya/issues/285 ?

DrDaleks commented 4 years ago

Basically, support for NaN was deemed illegal, although I hardly agree (according to IEEE standards), but issue was closed

andi-huber commented 4 years ago

Indriya is designed to provide arithmetic for finite quantities only. Whenever a calculation leaves the realm of finite numbers we fail early. I highly doubt, that we are going to change this in support of non finite quantities. (I don't regard this one an issue.)

DrDaleks commented 4 years ago

I honestly fail to understand why returning NaN is neither bad practice nor non IEEE-compilant... That would "legally" solve this corner case, which is not as unfrequent as it seems (we're at least two independent dev teams to have the issue). There are discussions on our side on calling a nogo for indriya because of this.

=> intercepting NaNs before deferring operations to the Calculator perhaps?

andi-huber commented 4 years ago

Can you share your particular use-case? Maybe there is a different solution.

DrDaleks commented 4 years ago

Rather simple use case really. Having a bunch of double arrays to convert on a regular basis throughout runtime, with some NaNs in it (for business related reasons). But instead of bulk converting it (with a simple lambda for instance), whichWe did before switching to indriya, we now have to stick in some isNaN everywhere to return NaN instead of calling the conversion, while the API could do it almost for free.

keilw commented 4 years ago

Well if the NaN has a domain specific reason, I can't say, if a very specific business case justifies that? Have you looked at other implementations like https://github.com/unitsofmeasurement/seshat (which I know so far still implements version 1.0) if they allow NaN value? @desruisseaux, can you answer that? The API does not do anything for free, only implementations may support it, but it's a question of how reasonable that is, and the JDK doesn't support it (see initial description as well as my remarks, BigDecimal does not support NaN) properly, so I don't think we can do something the JDK doesn't.

As far as I can see, you probably don't have to reinvent the wheel (aka create an entirely new JSR 385 implementation for your needs) but it looks like you may need your own implementation of NumberSystem.

andi-huber commented 4 years ago

I see, so you basically have numbers in your array that for some reason are also allowed to be NaN, which might correspond to the business meaning of a number/measurement being absent.

I'd personally explore the option of using Java's OptionalDouble to wrap these numbers, such that those that are non finite are represented by OptionalDouble.empty(). Your business logic/algorithm might then do unit transformation/calculation only if the OptionalDouble is present.

Why we don't support numbers representing NaN:

Agreed, there might be cases, where NaN support seems convenient, but I do also believe there are always alternative solutions here.

In theory I think, one could write an implementation of a NumberSystem with NaN support and plug it in as needed. Which of course ignores consistency with formatting and parsing.

fawidmer commented 4 years ago

Thank you for your answers. For completeness, I can tell you a little something about our usecase: We are a team of researchers at a ETH (Swiss university). We are dealing with data analysis and control algorithms which we are used to developing in Matlab. For our online application, however, we use Java.

To simplify the transfer of code to Java, we implemented an own library for Matlab-like n-dimensional tensor operations in Java. We're using Indriya to extend the functionalities of these tensors with units.

Thus, we are very used to the way Matlab treats NaN / Inf values:

In Matlab, it is common to use e.g. NaN in an array or tensor to represent non-present numbers, that's why we would like to have that as well in Java. We now implemented a workaround to make it work for our use case.

So if your decision is to not support NaN, you can close this issue if you want.

keilw commented 4 years ago

We already faced a lot of discussion and controversy trying to introduce "Mixed Quantities" also present in Wolfram Alpha or Matlab. Not every calculation that Matlab or similar products can do are supported in Java. And the root cause is actually BigDecimal not supporting it: @throws NumberFormatException if {@code val} is infinite or NaN. Therefore I don't think we can support it now. Can you check if other Java Number implementations like Apache Commons Numbers allow using NaN? Also see #254 where we asked about other number systems. As @andi-huber also mentioned, creating a NumberSystem for your own custom need sounds best, if a future update of the RI finds additional number systems and representations other than the current one heavily based on BigDecimal/BigInteger for higher precisions, then it could be easier to come back to this. I would defer it and not close it.

keilw commented 3 years ago

@andi-huber There has been little input or progress here, do you see a need to act or shall we close it for now?

andi-huber commented 3 years ago

Hi @keilw - I don't see any need to act. Closing it.