unitsofmeasurement / indriya

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

Angular units such as RADIAN and STERADIAN are dimensionless #257

Closed wnreynoldsWork closed 2 years ago

wnreynoldsWork commented 4 years ago

RADIAN and STERADIAN defined in Units.java are defined as being dimensionless. This leads to strange behavior like RADIAN.isCompatible(STERADIAN) == true.

I strongly urge that new dimensions be defined for RADIAN and STERADIAN (eg PLANE_ANGLE_DIMENSION and SOLID_ANGLE_DIMENSION).

In my code, I did the following:

// Custom angular units with custom dimensions to work around
// dimensionless indriya angular units
public final static Dimension PLANE_ANGLE_DIMENSION = UnitDimension.parse("\u2220".charAt(0));
public final static Dimension SOLID_ANGLE_DIMENSION = UnitDimension.parse("\u03A9".charAt(0));
public final static Unit<SolidAngle> STERADIAN = new BaseUnit<SolidAngle>("sr", SOLID_ANGLE_DIMENSION);
public final static Unit<Angle> RADIAN = new BaseUnit<Angle>("rad", PLANE_ANGLE_DIMENSION);

public static final Unit<Angle> REVOLUTION = RADIAN.multiply(2.0*M_PI);
public static final Unit<Angle> DEGREE_ANGLE = REVOLUTION.divide(360.0);
public static final Unit<Angle> MINUTE_ANGLE = DEGREE_ANGLE.divide(60.0);
public static final Unit<Angle> SECOND_ANGLE = MINUTE_ANGLE.divide(60.0);

Thank you for writing a library that enables this sort of extension.

On a sour note, I could not mirror the code in NonSI.java to construct the above units, since the various transformer classes in tech.units.indriya.function are all package private (eg PowerOfPiConverter and RationalConverter.of()). Consequently, I request these be made public so that new units can be easily defined.

andi-huber commented 4 years ago

1) If RADIAN.isCompatible(STERADIAN) == true comes as a surprise to you, we might need to clarify the associated java-doc, and find better words to describe what we mean here. If you are interested in checking whether 2 units are equivalent, this is a different story [1]. The prototypical example to determine unit equivalence is this

ComparableUnit a = (ComparableUnit) MICRO(GRAM);
ComparableUnit b = (ComparableUnit) GRAM.divide(1_000_000);
assertEquals(true, a.isEquivalentTo(b));
assertEquals(true, b.isEquivalentTo(a));

2) To instantiate different MultiplyConverters please have a look at the various factory methods in tech.units.indriya.function.MultiplyConverter. We have decided to make the MultiplyConverter implementations private, so that we have the freedom to change them or phase them out without notice.

[1] https://github.com/unitsofmeasurement/indriya/wiki/Unit-Equivalence

keilw commented 4 years ago

RADIAN is said to be Dimensionless https://en.wikipedia.org/wiki/Radian, but STERADIAN should be SolidAngle: https://en.wikipedia.org/wiki/Steradian

keilw commented 4 years ago

@wnreynoldsWork What do you suggest to change? Both RADIAN and STERADIAN are said to be "Dimensionless" by Wikipedia. They each have a Quantity type like Angle, but no Dimension. The special constant of NONE here. Inventing a new dimension for these two seems wrong and against the definition of these units.

wnreynoldsWork commented 4 years ago

Well, this is a bit of a rathole. We should be careful of the fallacy of arguing from authority. I feel there are arguments supporting both sides.

1) A key use of the uom package for me is determining whether two units are commensurate. I'm using the isCompatible() method to check this, which I understand compares the dimensions of the two units. W/sr is not the same as W, and the two should not be commensurate. As a practical matter, a software system should provide facilities to determine that such quantities are not commensurate.

2) The geometric definition of plane and solid angle imply no dimension arclength = angle radius, surface area = solid angle radius^2. However, when we make angle/solid angle dimensionless, this allows us to make mistakes like arclength = angle in degrees * radius.

So we have a paradox. Solving this paradox is definitely out of scope of this discussion (as an aside, I think the answer is that arclength of a circle and surface area on sphere of radius r are not the same as 'length' and 'area', similar to torque not being the same as energy).

If you agree with my point (1), then you agree that a UOM software system should has requirements both to distinguish units with angular components and and to go from angle->arclength and solidangle->surface area. So how to fix that?

1) Make the angular units dimensioned, and forever field questions from folks who are confused that radius*angle != length (Not recommended).

2) Fix it in the documentation. In the in-development user guide, bring up these issues, and provide example code like the above to show how to implement dimensional versions of the angular units (and again kudos for good package design, and having reviewed the code in MultiplyConverter I withdraw my nit about the package-private implementations).

3) Go a step further, in addition to the documentation, provide dimensioned versions of the angular units, eg RADIAN_DIMENSIONED.

4) Introduce a new ConvertibleUnit type that allows switching between dimensioned and non-dimensioned versions. (Also not recommended).

As usual, I'd be happy to contribute some stuff if it would help.

keilw commented 4 years ago

I see problems with 3 and 4 as to making things more complicated. We already have a necessary evil of such kind with the "scale" of a quantity to deal with absolute vs. relative quantities. About RADIAN Wikipedia https://en.wikipedia.org/wiki/Radian#Dimensional_analysis says "Although the radian is a unit of measure, it is a dimensionless quantity." So I would not make things up where no scientific or mathematical definitions exist. With Celcius vs. Fahrenheit there are "absolute" vs. "relative" temperatures, therefore this new attribute whatever complications it may cause was useful.

You're welcome to contribute beyond such issue tickets, but this area especially the RI means one has to be at least a JCP Associate member. Did you already consider that?

wnreynoldsWork commented 4 years ago

I would be caution against arguments such as 'wikipedia says', but I agree that convention amongst users is an important consideration. Arguing further on the nature of a radian is probably out of scope (although interesting).

I did consider a JCP membership, but sadly, my organization is not interested in spending several thousand dollars so I can contribute code.

I'd be happy to write up a page or so and post it here or send it to you if you are interested.

keilw commented 4 years ago

It's free for all now, when did your company last look at that? Even if the burden of going to your boss for the JSPA was an issue, why not join as an Associate Member? Check out https://jcp.org/en/participation/membership I know Wikipedia may not be the universal source of truth, but if there's a statement like "XYZ is a dimensionless unit" that is usually backed by something like the BIPM and SI Brochure, which is the definitive source for us.

wnreynoldsWork commented 4 years ago

I agree that community standards are important. I wanted to make sure there were substantive arguments for and against, and I think there are. A quick google unearths a good conversation here: https://physics.stackexchange.com/questions/252288/are-units-of-angle-really-dimensionless/252292. I enjoy the quote: "that dimensional status is to a degree decided by convention rather than by fact". The initial poster came to the exact conclusion I did - you need dimensions on angular units if you are going to do dimensional analysis. In any case, the argument is out of scope here. The important thing is that uom is able to support this use case, which it is.

I'll look into a membership and try to do a writeup.

keilw commented 4 years ago

Well the discussion even pointed to an earlier update of the BIPM: https://www.bipm.org/en/CGPM/db/20/8/ Sure, you're welcome, if you join within a few weeks you would even be eligible to vote on the next JCP EC (where a few actively involved in JSR 385 are among the candidates ;-)

wnreynoldsWork commented 4 years ago

Yup, its a standard, but as the (really quite thoughtful) discussion indicated, it can make sense to have angles dimensioned, especially in a software library, and in fact Boost, for example, does this. Anyways, horse is dead and beaten to hamburger.

keilw commented 4 years ago

Boost, where is that? And what is the language it uses? The UCUM catalog also has a different definition of GRAM and KILOGRAM compared to the SI. It is however a little outdated now and seems little maintained over the last couple of years.

keilw commented 4 years ago

C++ I think I heard about it somewhere. Is there a Github repo for it?

wnreynoldsWork commented 4 years ago

Boost is a massive C++ library providing tons of functionality, including uom, https://www.boost.org/. It's deeply embedded in the C++ world, certainly in my organization.

wnreynoldsWork commented 4 years ago

https://www.boost.org/doc/libs/1_71_0/doc/html/boost_units.html

keilw commented 4 years ago

Interesting, in fact quantity<length> L = 2.0*meters; sounds not so different from how we can phrase that in Java, with a bit of Kotlin maybe ;-)

wnreynoldsWork commented 4 years ago

From the Boost Units FAQ: Angles are treated as units If you don't like this, you can just ignore the angle units and go on your merry way (periodically screwing up when a routine wants degrees and you give it radians instead...)

https://www.boost.org/doc/libs/1_71_0/doc/html/boost_units/FAQ.html#boost_units.FAQ.Angle_Are_Units

:-)

keilw commented 4 years ago

If you look at https://www.boost.org/doc/libs/1_71_0/doc/html/boost_units/Examples.html it does however have an example:

Defining a few angular quantities,

/// test trig stuff
quantity<plane_angle>           theta = 0.375*radians;
quantity<dimensionless>         sin_theta = sin(theta);
quantity<plane_angle>           thetap = asin(sin_theta);

yields

theta            = 0.375 rd
sin(theta)       = 0.366273 dimensionless
asin(sin(theta)) = 0.375 rd

where the results are also mentioned as being dimensionless. Sin and Asin I don't know, where we already used that for quantities? java.Math offers basic sin() and asin() methods. Guess it's something we could try adding with Lambdas.

keilw commented 4 years ago

@dautelle or @desruisseaux can maybe provide a bit more input here, but of the top of my head the "homogeneous systems vs. heterogeneous systems" feels like applying a different DimensionalModel

desruisseaux commented 4 years ago

For preventing RADIAN to be mixed with STERADIAN, the getSystemUnit() method may be more useful than the Unit.isCompatible(Unit) method (actually I'm not sure what are the use cases for Unit.isCompatible(Unit)):

public static boolean isSameQuantity(Unit<?> a, Unit<?> b) {
    return a.getSystemUnit().equals(b.getSystemUnit());}
}

I agree that Wikipedia should be taken carefully, but BIPM is the result of international agreements; it is the most authoritative source I'm aware of, and the source of other standardisation bodies like NIST. Admittedly the SI dimensions result from human decisions rather than fundamental laws of nature, but BIPM worked hard in trying to come with a consistent set of dimensions. This choice does not resolve all problems (e.g. Torque has the same dimension than Energy, so those two quantities suffer from the same problem than RADIAN versus STERADIAN) but I suspect that no set of dimensions would be perfect.

In the particular case of radians, I think it should stay dimensionless. Otherwise as noted above, angle * radius would not have length dimension; we would have a new, different, quantity for arc length measured on a circle. Then what about lengths measured on other shapes, e.g. what should be the dimension of arc length on ellipse? Or the length of a coast line?

It may not be a convincing argument, but as a side note, series expansions like sin would not be allowed if RADIAN was not dimensionless, because additions of x raised to different powers (x³, x⁵, x⁷, ...) would not be permitted:

sin(x) = x - x³/6 + x⁵/120 - x⁷/5040 + ...

But as said above, having RADIAN dimensionless does not prevent us from using a different Quantity class and from differentiating radians from steradians by comparing getSystemUnit(). One open question however could be: is Unit.isCompatible(Unit) really convenient the way it is currently defined? Shouldn't we compare getSystemUnit() instead of getDimension()?

keilw commented 4 years ago

Thanks for the quick reply @desruisseaux. I guess especially to refine the result if getDimension() is equal getSystemUnit() should probably be used.

wnreynoldsWork commented 4 years ago

Example use case for isCompatible() - We've implemented an expression + unit parser for our application. Say a user enters the string:

1 {meter} + 1 {foot}

meter.isCompatible(foot) == true, so the expression is allowed. It is parsed and stored into the system for execution by our system.

If the user enters

1 {meter} + 1 {second}

meter.isCompatible(second) == false, so an error message is displayed and the expression is rejected.

desruisseaux commented 4 years ago

@wnreynoldsWork thanks, but my question is rather: what would be the use case of a isCompatible contract based on comparisons of getDimension() instead of getSystemUnit()?

What is the use case for the former instead of the later?

wnreynoldsWork commented 4 years ago

I use isCompatible() for for determining if two quantities are commensurate, ie whether they have the same dimension, independent of whether they use the same units or not, I described the use case for that above. For dimensioned units, it matches my use case perfectly. Dimensionless units screw things up.

So I guess you're saying that even when getCompatible() returns true, I should do a second check to see if things are compatible with ONE (ie dimensionless) and then do a third check comparing if both units are the same getSystemUnit(). But I don't think this works for my use case. For example "1 {degree} + 1 {radian}" should be allowed but would not be in this scheme. Having dimensioned units throughout does solve the problem nicely. And I can have them by defining my own.

Regarding the series expansion argument, I think you forget that your using a Taylor series, and the coefficients are the higher order derivatives, ie - the n'th coefficient is really 1/n! d^n sin(x)/dx^n *x^n, so the units cancel out and each term is dimensionless. The physics.stackexchange link mentioned above has a pretty good discussion of how functions impact the use of units (eg how to handle sin(x) = (e^ix - e^-x)/2i). My guess is that units will eventually come out in the wash, and everything works fine. (Again, fun discussion, but probably out of scope. ).

I think I have a valid use case for isCompatible() that is not easily solved by other tests, but is solved by dimensioned units. While I'm sure we could come up with some baroque ways to distinguish between RADIAN and STERADIAN (and their absence: we'd need to catch (W/sr).isCompatible(W) == true), moving to dimensioned angles is a much more elegant approach.

I agree with your statement that units are conventions, and that no one convetion suits everybody's needs. It is probably not worth it to break your current design to accommodate dimensioned angles, but it's nice to be able to add them in.

desruisseaux commented 4 years ago

There is no need to perform all those checks (test if isCompatible return true, test if ONE, etc). Doing only the following test should be sufficient for all cases - you can forget about isCompatible and ONE:

u1.getSystemUnit().equals(u2.getSystemUnit())

It will work with "1 degree + 1 radian" because those two units have "radian" as their system unit. It will differentiate "1 radian" from "1 steradian" because those two units have different system units.

The current contract of isCompatible indeed said if two units have the same dimensions. But my question about use case is because I think that most of the time, we want to verify if two units are used for the same quantity, which is a different thing than having the same dimensions. So I wonder if there is use cases where someone wants to verify if two units have the same dimensions but not necessarily used for the same quantity.

wnreynoldsWork commented 4 years ago

PrintStream out = new PrintStream(System.out, true, "utf-8"); Unit<?> rad = Units.RADIAN; Unit<?> deg = NonSI.DEGREE_ANGLE; Unit<?> sr = Units.STERADIAN;

    out.println("rad = " + rad + " degree = " + deg + " compatible = " + rad.isCompatible(deg));
    out.println("rad = " + rad + " degree = " + deg + " same system unit  = " + rad.getSystemUnit().equals(deg.getSystemUnit()));
    out.println("rad = " + rad + " sr = " + sr + " compatible = " + rad.isCompatible(sr));
    out.println("rad = " + rad + " sr = " + sr + " same system unit  = " + rad.getSystemUnit().equals(sr.getSystemUnit()));

    Unit<?> watt = Units.WATT;
    Unit<?> wattPerRad = watt.divide(rad);
    Unit<?> wattPerSr = watt.divide(sr);
    out.println("wattPerRad = " + wattPerRad + " wattPerSr = " + wattPerSr + " compatible = " + wattPerRad.isCompatible(wattPerSr));
    out.println("wattPerRad = " + wattPerRad + " wattPerSr = " + wattPerSr + " same system unit = " + wattPerRad.getSystemUnit().equals(wattPerSr.getSystemUnit()));

Output: rad = rad degree = deg compatible = true rad = rad degree = deg same system unit = true rad = rad sr = sr compatible = true rad = rad sr = sr same system unit = false wattPerRad = W/rad wattPerSr = W/sr compatible = true wattPerRad = W/rad wattPerSr = W/sr same system unit = false

so it seems to work!

wnreynoldsWork commented 4 years ago

Unit<?> meterPerDegPerSecondPerBar = Units.METRE.divide(deg).divide(Units.SECOND).divide(NonSI.BAR); Unit<?> meterPerSrPerSecondPerBar = Units.METRE.divide(sr).divide(Units.SECOND).divide(NonSI.BAR);

    out.println(meterPerDegPerSecondPerBar.getSystemUnit()  );
    out.println(meterPerSrPerSecondPerBar.getSystemUnit()  );
    out.println(meterPerDegPerSecondPerBar.isCompatible(meterPerSrPerSecondPerBar));
    out.println(meterPerDegPerSecondPerBar.getSystemUnit().equals(meterPerSrPerSecondPerBar.getSystemUnit()));

m/(Pa·rad·s) m/(Pa·s·sr) true false

so that might work. Maybe replace isCompatible() with this as an implementation?

desruisseaux commented 4 years ago

The problem is that current behaviour is specified by javadoc, so it is part of the method contract. It seems to me that changing the contract (before to change the implementation) would be a JSR change.

Why the contract has been defined that way in the first place (why the javadoc said that isCompatible implementers have to compare dimensions instead than comparing system units), I have no idea. This is why I asked what would be the use cases in previous comments.

wnreynoldsWork commented 4 years ago

perhaps a new method, say isSystemUnitCompatible(Unit<?> otherUnit)? If added, I'd recommend a pointer to this method from the isCompatabile() javadoc to prevent dimensionless surprises.

wnreynoldsWork commented 4 years ago

Ok, first problem with this approach:

    Unit<?> joule = Units.JOULE;
    Unit<?> newton = Units.NEWTON;
    Unit<?> newtonMeter = Units.NEWTON.multiply(Units.METRE);
    out.println(joule.getSystemUnit().equals(newtonMeter.getSystemUnit()));

output: false.

I can try my tests with something like isCompatible() || gSU.equals(gSU)

wnreynoldsWork commented 4 years ago

Ok, this winds up being a deal-breaker for me due to the technical way in which I manage lookups for units. I'd been storing units in as lookup table keyed on dimension, which worked well with dimensioned angular units. I could probably get the lookup to work with several levels of checks using getSystemUnit(), but the complexity is not justified, especially given that the dimensioned approach works very well.

desruisseaux commented 4 years ago

The JOULE versus NEWTON issue reported above may be an Indriya implementation issue. The same test executed on Seshat returns true as expected. I think that what have been said before about comparing getSystemUnit() stay true in principle, but some implementations may need fixes.

keilw commented 4 years ago

Can you point to a particular class and propose a PR here based on that? We just finished 385 (and were awarded for it, as well;), but even as a MR in that case (which may also allow minor API changes) certain special cases involving physical specs and standards might be a nice addition to the TCK in particular areas, at least when the SI is involved, because it's a tricky area that requires knowledge about a particular implementation, so not sure, if it's reasonable for the TCK which so far checks precisely how all interfaces are implemented, not mathematical operations or if the "Celsius Problem" is solved ;-)

keilw commented 4 years ago

Was there any conclusion or action item here? E.g. has a ticket been created for a possible NEWTON issue?

keilw commented 3 years ago

@desruisseaux You were the last to comment on this over 9 months ago, do you see any action item here or can it be closed?

desruisseaux commented 3 years ago

I have not done real analysis… The issue reported here seems valid to me, and it seems specific to Indriya implementation. So it may be a matter of running a test case in a debugger session. Maybe this is an issue that could be flagged with a "Help wanted" tag?

keilw commented 3 years ago

As the newer #322 was closed already I am inclined to think this is also old and probably should be closed.

desruisseaux commented 3 years ago

Yes, I did not verified if this issue is still present today. I have no objection for closing (or other action at your choice).

keilw commented 3 years ago

I'll check against actual changes in #322, thanks for the input.

keilw commented 2 years ago

There is nothing more do do here, a possible improvement of Newton-metre is a separate ticket #380.