Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UCUM parsing "Hz/60" != Units.HERTZ.divide(60) #151

Open
msqr opened this issue Sep 26, 2019 · 2 comments
Open

UCUM parsing "Hz/60" != Units.HERTZ.divide(60) #151

msqr opened this issue Sep 26, 2019 · 2 comments
Labels

Comments

@msqr
Copy link

msqr commented Sep 26, 2019

Hello, I was writing some tests to verify parsing of UCUM units, and ran into a problem where parsing Hz/60 did not result in a Unit that was equal to Units.HERTZ.divide(60) as I was expecting. When parsing the string, the result ends up as Hz/one*60. I wondered if these two Units should actually be considered equal? Maybe this is related to #140?

Here's some example test code of what I am doing:

public void ucum_rpm() throws UnconvertibleException, IncommensurableException {
    UCUMServiceProvider sp = new UCUMServiceProvider();

    // the following returns the DefaultFormatService class
    UnitFormat uf = sp.getFormatService().getUnitFormat();
    Unit<?> unit = uf.parse("Hz/60");
    assertThat("Unit is RPM", unit, equalTo(Units.HERTZ.divide(60)));

    // the following returns the UCUMFormatService class
    UnitFormat uf2 = sp.getUnitFormatService().getUnitFormat();
    Unit<?> unit2 = uf2.parse("Hz/60");

    // as an alternative check, look for identify transform
    UnitConverter conv = unit2.getConverterToAny(Units.HERTZ.divide(60));
    assertThat("Unit conversion to RPM is identity", conv.isIdentity(), equalTo(true));

    // The following assert does not work because `unit` is parsed into Hz/one*60
    assertThat("Unit is RPM", unit2, equalTo(Units.HERTZ.divide(60)));
}
@keilw
Copy link
Member

keilw commented Sep 27, 2019

With a quantity it is often the case, that a rather strict equals() method by the JDK fails because it looks at the exact class which is not representative in many cases, that's why we introduced the notion of equivalence. It may happen less often with units, but a TransformedUnit and AlternateUnit are also not equal in the sense of Java, so it is likely because of that. Did you try isEquivalentTo() here as well? Aside from that, while HERTZ has no special character in the UCUM catalog, you may stick to one of the classic UnitFormat implementations for units in the RI, the UCUMFormat you got from the UCUMServiceProvider is optimized to work with the UCUM system.

@keilw keilw added the analysis label Sep 27, 2019
@msqr
Copy link
Author

msqr commented Sep 27, 2019

If I cast the Unit to a ComparableUnit then isEquivalentTo() does return true -- I did this:

final Unit<Frequency> rpm = Units.HERTZ.divide(60);
if ( unit2.isCompatible(rpm) && unit2 instanceof ComparableUnit<?> ) {
    ComparableUnit<Frequency> cu2 = (ComparableUnit<Frequency>) unit2.asType(Frequency.class);
    assertThat("Unit is RPM", cu2.isEquivalentTo(rpm), equalTo(true));
}

Unfortunately ComparableUnit and thus isEquivalentTo() is not in the javax.measure API, that is why I was using equals() for comparison. The UCUMServiceProvider is used here as I extracted the test case out of an application that is relying on UCUM. Is there a recommended way to compare Unit instances for equality while staying within the javax.measure API? For example using UnitConverter.isIdentity() worked, but I wondered if there was a more straightforward way?

msqr added a commit to SolarNetwork/solarnetwork-node that referenced this issue Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants