[VOTE] Release Apache Commons Geometry (full distribution) 1.0-beta1 based on RC1

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

[VOTE] Release Apache Commons Geometry (full distribution) 1.0-beta1 based on RC1

Matt Juntunen
We have been working hard to prepare Apache Commons Geometry for an initial release, so I would like to release Apache Commons Geometry (full distribution) 1.0-beta1.

Apache Commons Geometry (full distribution) 1.0-beta1 RC1 is available for review here:
    https://dist.apache.org/repos/dist/dev/commons/geometry/1.0-beta1-RC1 (svn revision 40275)

The Git tag commons-geometry-1.0-beta1-rc1 commit for this RC is cc94f654e13954c805650d6ab9f6bee333c75a31 which you can browse here:
    https://gitbox.apache.org/repos/asf?p=commons-geometry.git;a=commit;h=cc94f654e13954c805650d6ab9f6bee333c75a31

You may checkout this tag using:
    git clone https://gitbox.apache.org/repos/asf/commons-geometry.git --branch commons-geometry-1.0-beta1-rc1 commons-geometry-1.0-beta1-rc1

Maven artifacts are here:
    https://repository.apache.org/content/repositories/orgapachecommons-1502/org/apache/commons/

These are the artifacts and their hashes:

commons-geometry-1.0-beta1-bin.tar.gz=373ea50d5da28c3ba64dc15058d55dce333ee04a9664d5b69605977271d89d403ac854afb1f6d8108e803e8d6860aae2ae3a9f83fc153b5855b0427bff05b6f4
commons-geometry-1.0-beta1-bin.zip=329cfbd631b7f403f4329b77f564e5aca702746764c23e35756a3100a58ea6896b38b499f3a68cd1bea5d0a72fd63f157e7aeced0dc6377c77d417142b0b4355
commons-geometry-1.0-beta1-src.tar.gz=5ffda78c3a8c2e03e33929a615dcd35181809b85c7aaf1f719a4d51ab4d48f485022707327d4c26c345f902b0c563ab7e5c4d1c00c76823e7d25ba1aa6bc8a20
commons-geometry-1.0-beta1-src.zip=1e6cecf35e5e952cd267463e22a9be00ac0cb80231ee87e475d643d943b4915421f08ca6f15ece51c687c6656acabf5161294f5d3a9dc025fde2657d5f8a27f3

(no need for .asc hashes!)

I have tested this with ***'mvn clean install site'*** using:
***
Apache Maven 3.5.3 (3383c37e1f9e9b3bc3df5050c29c8aff9f295297; 2018-02-24T14:49:05-05:00)
Maven home: /home/matt/tools/maven/apache-maven-3.5.3
Java version: 1.8.0_252, vendor: Private Build
Java home: /usr/lib/jvm/java-8-openjdk-amd64/jre
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "4.15.0-106-generic", arch: "amd64", family: "unix"
***

Details of changes are in the release notes:
    https://dist.apache.org/repos/dist/dev/commons/geometry/1.0-beta1-RC1/RELEASE-NOTES.txt
    https://home.apache.org/~mattjuntunen/commons-geometry-1.0-beta1-rc1-site/changes-report.html

Site:
    https://home.apache.org/~mattjuntunen/commons-geometry-1.0-beta1-rc1-site/index.html
    (note some *relative* links are broken and the 1.0-beta1 directories are not yet created - these will be OK once the site is deployed.)

CLIRR Report:
    N/A

JApiCmp Report:
    N/A

RAT Report:
    https://home.apache.org/~mattjuntunen/commons-geometry-1.0-beta1-rc1-site/rat-report.html

KEYS:
  https://www.apache.org/dist/commons/KEYS

Please review the release candidate and vote.
This vote will close no sooner that 72 hours from now.

  [ ] +1 Release these artifacts
  [ ] +0 OK, but...
  [ ] -0 OK, but really should fix...
  [ ] -1 I oppose this release because...

Thank you,

Matt Juntunen,
Release Manager (using key 7DD53AEFEDF1C3D392B51EBE346F4FCECFB70B1A)

The following is intended as a helper and refresher for reviewers.

Validating a release candidate
==============================

These guidelines are NOT complete.

Requirements: Git, Java, Maven.

You can validate a release from a release candidate (RC) tag as follows.

1) Clone and checkout the RC tag

git clone https://gitbox.apache.org/repos/asf/commons-geometry.git --branch commons-geometry-1.0-beta1-RC1 commons-geometry-1.0-beta1-RC1
cd commons-geometry-1.0-beta1-RC1

2) Check Apache licenses

This step is not required if the site includes a RAT report page which you then must check.

mvn apache-rat:check

3) Check binary compatibility

Older components still use Apache Clirr:

This step is not required if the site includes a Clirr report page which you then must check.

mvn clirr:check

Newer components use JApiCmp with the japicmp Maven Profile:

This step is not required if the site includes a JApiCmp report page which you then must check.

mvn install -DskipTests -P japicmp japicmp:cmp

4) Build the package

mvn -V clean package

You can record the Maven and Java version produced by -V in your VOTE reply.
To gather OS information from a command line:
Windows: ver
Linux: uname -a

5) Build the site for a single module project

Note: Some plugins require the components to be installed instead of packaged.

mvn site
Check the site reports in:
- Windows: target\site\index.html
- Linux: target/site/index.html

6) Build the site for a multi-module project

mvn site
mvn site:stage
Check the site reports in:
- Windows: target\site\index.html
- Linux: target/site/index.html

Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Release Apache Commons Geometry (full distribution) 1.0-beta1 based on RC1

Sven Rathgeber
+1

Thanks for tons of work !!!

Sven

On 7/3/20 7:34 AM, Matt Juntunen wrote:

> We have been working hard to prepare Apache Commons Geometry for an initial release, so I would like to release Apache Commons Geometry (full distribution) 1.0-beta1.
>
> Apache Commons Geometry (full distribution) 1.0-beta1 RC1 is available for review here:
>      https://dist.apache.org/repos/dist/dev/commons/geometry/1.0-beta1-RC1 (svn revision 40275)
>
> The Git tag commons-geometry-1.0-beta1-rc1 commit for this RC is cc94f654e13954c805650d6ab9f6bee333c75a31 which you can browse here:
>      https://gitbox.apache.org/repos/asf?p=commons-geometry.git;a=commit;h=cc94f654e13954c805650d6ab9f6bee333c75a31
>
> You may checkout this tag using:
>      git clone https://gitbox.apache.org/repos/asf/commons-geometry.git --branch commons-geometry-1.0-beta1-rc1 commons-geometry-1.0-beta1-rc1
>
> Maven artifacts are here:
>      https://repository.apache.org/content/repositories/orgapachecommons-1502/org/apache/commons/
>
> These are the artifacts and their hashes:
>
> commons-geometry-1.0-beta1-bin.tar.gz=373ea50d5da28c3ba64dc15058d55dce333ee04a9664d5b69605977271d89d403ac854afb1f6d8108e803e8d6860aae2ae3a9f83fc153b5855b0427bff05b6f4
> commons-geometry-1.0-beta1-bin.zip=329cfbd631b7f403f4329b77f564e5aca702746764c23e35756a3100a58ea6896b38b499f3a68cd1bea5d0a72fd63f157e7aeced0dc6377c77d417142b0b4355
> commons-geometry-1.0-beta1-src.tar.gz=5ffda78c3a8c2e03e33929a615dcd35181809b85c7aaf1f719a4d51ab4d48f485022707327d4c26c345f902b0c563ab7e5c4d1c00c76823e7d25ba1aa6bc8a20
> commons-geometry-1.0-beta1-src.zip=1e6cecf35e5e952cd267463e22a9be00ac0cb80231ee87e475d643d943b4915421f08ca6f15ece51c687c6656acabf5161294f5d3a9dc025fde2657d5f8a27f3
>
> (no need for .asc hashes!)
>
> I have tested this with ***'mvn clean install site'*** using:
> ***
> Apache Maven 3.5.3 (3383c37e1f9e9b3bc3df5050c29c8aff9f295297; 2018-02-24T14:49:05-05:00)
> Maven home: /home/matt/tools/maven/apache-maven-3.5.3
> Java version: 1.8.0_252, vendor: Private Build
> Java home: /usr/lib/jvm/java-8-openjdk-amd64/jre
> Default locale: en_US, platform encoding: UTF-8
> OS name: "linux", version: "4.15.0-106-generic", arch: "amd64", family: "unix"
> ***
>
> Details of changes are in the release notes:
>      https://dist.apache.org/repos/dist/dev/commons/geometry/1.0-beta1-RC1/RELEASE-NOTES.txt
>      https://home.apache.org/~mattjuntunen/commons-geometry-1.0-beta1-rc1-site/changes-report.html
>
> Site:
>      https://home.apache.org/~mattjuntunen/commons-geometry-1.0-beta1-rc1-site/index.html
>      (note some *relative* links are broken and the 1.0-beta1 directories are not yet created - these will be OK once the site is deployed.)
>
> CLIRR Report:
>      N/A
>
> JApiCmp Report:
>      N/A
>
> RAT Report:
>      https://home.apache.org/~mattjuntunen/commons-geometry-1.0-beta1-rc1-site/rat-report.html
>
> KEYS:
>    https://www.apache.org/dist/commons/KEYS
>
> Please review the release candidate and vote.
> This vote will close no sooner that 72 hours from now.
>
>    [ ] +1 Release these artifacts
>    [ ] +0 OK, but...
>    [ ] -0 OK, but really should fix...
>    [ ] -1 I oppose this release because...
>
> Thank you,
>
> Matt Juntunen,
> Release Manager (using key 7DD53AEFEDF1C3D392B51EBE346F4FCECFB70B1A)
>
> The following is intended as a helper and refresher for reviewers.
>
> Validating a release candidate
> ==============================
>
> These guidelines are NOT complete.
>
> Requirements: Git, Java, Maven.
>
> You can validate a release from a release candidate (RC) tag as follows.
>
> 1) Clone and checkout the RC tag
>
> git clone https://gitbox.apache.org/repos/asf/commons-geometry.git --branch commons-geometry-1.0-beta1-RC1 commons-geometry-1.0-beta1-RC1
> cd commons-geometry-1.0-beta1-RC1
>
> 2) Check Apache licenses
>
> This step is not required if the site includes a RAT report page which you then must check.
>
> mvn apache-rat:check
>
> 3) Check binary compatibility
>
> Older components still use Apache Clirr:
>
> This step is not required if the site includes a Clirr report page which you then must check.
>
> mvn clirr:check
>
> Newer components use JApiCmp with the japicmp Maven Profile:
>
> This step is not required if the site includes a JApiCmp report page which you then must check.
>
> mvn install -DskipTests -P japicmp japicmp:cmp
>
> 4) Build the package
>
> mvn -V clean package
>
> You can record the Maven and Java version produced by -V in your VOTE reply.
> To gather OS information from a command line:
> Windows: ver
> Linux: uname -a
>
> 5) Build the site for a single module project
>
> Note: Some plugins require the components to be installed instead of packaged.
>
> mvn site
> Check the site reports in:
> - Windows: target\site\index.html
> - Linux: target/site/index.html
>
> 6) Build the site for a multi-module project
>
> mvn site
> mvn site:stage
> Check the site reports in:
> - Windows: target\site\index.html
> - Linux: target/site/index.html
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Release Apache Commons Geometry (full distribution) 1.0-beta1 based on RC1

Xeno Amess
I might review it tonight if I have time...
Only if I have time.
Really busy these days...
Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Release Apache Commons Geometry (full distribution) 1.0-beta1 based on RC1

Xeno Amess
Hi.
I see somethings not quite right already.
Will start pr directly.

Xeno Amess <[hidden email]> 于2020年7月3日周五 下午4:20写道:

> I might review it tonight if I have time...
> Only if I have time.
> Really busy these days...
>
Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Release Apache Commons Geometry (full distribution) 1.0-beta1 based on RC1

Xeno Amess
Hi.
Please check https://github.com/apache/commons-geometry/pull/84
In short, there be too many errors in javadoc.
(and this is the reason why I made
https://github.com/apache/commons-parent/pull/7)

There still exist some other problems, but I'd prefer check them after this
pr, as resolve conflict is boring.


Xeno Amess <[hidden email]> 于2020年7月4日周六 上午12:13写道:

> Hi.
> I see somethings not quite right already.
> Will start pr directly.
>
> Xeno Amess <[hidden email]> 于2020年7月3日周五 下午4:20写道:
>
>> I might review it tonight if I have time...
>> Only if I have time.
>> Really busy these days...
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Release Apache Commons Geometry (full distribution) 1.0-beta1 based on RC1

Xeno Amess
Hi.
Please also consider about
https://github.com/apache/commons-geometry/pull/85

I will perform more analyze opun this repo when you solve this two prs.

Xeno Amess <[hidden email]> 于2020年7月4日周六 上午1:01写道:

> Hi.
> Please check https://github.com/apache/commons-geometry/pull/84
> In short, there be too many errors in javadoc.
> (and this is the reason why I made
> https://github.com/apache/commons-parent/pull/7)
>
> There still exist some other problems, but I'd prefer check them after
> this pr, as resolve conflict is boring.
>
>
> Xeno Amess <[hidden email]> 于2020年7月4日周六 上午12:13写道:
>
>> Hi.
>> I see somethings not quite right already.
>> Will start pr directly.
>>
>> Xeno Amess <[hidden email]> 于2020年7月3日周五 下午4:20写道:
>>
>>> I might review it tonight if I have time...
>>> Only if I have time.
>>> Really busy these days...
>>>
>>
Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Release Apache Commons Geometry (full distribution) 1.0-beta1 based on RC1

Xeno Amess
Hi.
I see in some places of the repo, two double fields are compared using ==.
Shouldn't we use Double.compare?
Or is it by design?

example places:

PolarCoordinates.equals
SphericalCoordinates.equals
Vector1D.angle
Vector2D.equals
Vector3D.equals
Vectors.isRealNonZero

Xeno Amess <[hidden email]> 于2020年7月4日周六 上午1:51写道:

> Hi.
> Please also consider about
> https://github.com/apache/commons-geometry/pull/85
>
> I will perform more analyze opun this repo when you solve this two prs.
>
> Xeno Amess <[hidden email]> 于2020年7月4日周六 上午1:01写道:
>
>> Hi.
>> Please check https://github.com/apache/commons-geometry/pull/84
>> In short, there be too many errors in javadoc.
>> (and this is the reason why I made
>> https://github.com/apache/commons-parent/pull/7)
>>
>> There still exist some other problems, but I'd prefer check them after
>> this pr, as resolve conflict is boring.
>>
>>
>> Xeno Amess <[hidden email]> 于2020年7月4日周六 上午12:13写道:
>>
>>> Hi.
>>> I see somethings not quite right already.
>>> Will start pr directly.
>>>
>>> Xeno Amess <[hidden email]> 于2020年7月3日周五 下午4:20写道:
>>>
>>>> I might review it tonight if I have time...
>>>> Only if I have time.
>>>> Really busy these days...
>>>>
>>>
Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Release Apache Commons Geometry (full distribution) 1.0-beta1 based on RC1

Xeno Amess
Hi.
I finished the review.
Suggestions details are listed on
https://github.com/apache/commons-geometry/pulls
Notice that I am not familiar to this repo, nor a math expert, thus you
should not fully trust my prs, you need to see them, and check if every
change correct, because sometimes I make mistakes.

Xeno Amess <[hidden email]> 于2020年7月4日周六 上午10:09写道:

> Hi.
> I see in some places of the repo, two double fields are compared using ==.
> Shouldn't we use Double.compare?
> Or is it by design?
>
> example places:
>
> PolarCoordinates.equals
> SphericalCoordinates.equals
> Vector1D.angle
> Vector2D.equals
> Vector3D.equals
> Vectors.isRealNonZero
>
> Xeno Amess <[hidden email]> 于2020年7月4日周六 上午1:51写道:
>
>> Hi.
>> Please also consider about
>> https://github.com/apache/commons-geometry/pull/85
>>
>> I will perform more analyze opun this repo when you solve this two prs.
>>
>> Xeno Amess <[hidden email]> 于2020年7月4日周六 上午1:01写道:
>>
>>> Hi.
>>> Please check https://github.com/apache/commons-geometry/pull/84
>>> In short, there be too many errors in javadoc.
>>> (and this is the reason why I made
>>> https://github.com/apache/commons-parent/pull/7)
>>>
>>> There still exist some other problems, but I'd prefer check them after
>>> this pr, as resolve conflict is boring.
>>>
>>>
>>> Xeno Amess <[hidden email]> 于2020年7月4日周六 上午12:13写道:
>>>
>>>> Hi.
>>>> I see somethings not quite right already.
>>>> Will start pr directly.
>>>>
>>>> Xeno Amess <[hidden email]> 于2020年7月3日周五 下午4:20写道:
>>>>
>>>>> I might review it tonight if I have time...
>>>>> Only if I have time.
>>>>> Really busy these days...
>>>>>
>>>>
Reply | Threaded
Open this post in threaded view
|

Re: [VOTE] Release Apache Commons Geometry (full distribution) 1.0-beta1 based on RC1

Matt Juntunen
Thank you for the thorough review, Xeno! I'm pulling those changes in now.
-Matt
________________________________
From: Xeno Amess <[hidden email]>
Sent: Friday, July 3, 2020 11:17 PM
To: Commons Developers List <[hidden email]>; Matt Juntunen <[hidden email]>
Subject: Re: [VOTE] Release Apache Commons Geometry (full distribution) 1.0-beta1 based on RC1

Hi.
I finished the review.
Suggestions details are listed on https://github.com/apache/commons-geometry/pulls
Notice that I am not familiar to this repo, nor a math expert, thus you should not fully trust my prs, you need to see them, and check if every change correct, because sometimes I make mistakes.

Xeno Amess <[hidden email]<mailto:[hidden email]>> 于2020年7月4日周六 上午10:09写道:
Hi.
I see in some places of the repo, two double fields are compared using ==.
Shouldn't we use Double.compare?
Or is it by design?

example places:

PolarCoordinates.equals
SphericalCoordinates.equals
Vector1D.angle
Vector2D.equals
Vector3D.equals
Vectors.isRealNonZero

Xeno Amess <[hidden email]<mailto:[hidden email]>> 于2020年7月4日周六 上午1:51写道:
Hi.
Please also consider about https://github.com/apache/commons-geometry/pull/85

I will perform more analyze opun this repo when you solve this two prs.

Xeno Amess <[hidden email]<mailto:[hidden email]>> 于2020年7月4日周六 上午1:01写道:
Hi.
Please check https://github.com/apache/commons-geometry/pull/84
In short, there be too many errors in javadoc.
(and this is the reason why I made https://github.com/apache/commons-parent/pull/7)

There still exist some other problems, but I'd prefer check them after this pr, as resolve conflict is boring.


Xeno Amess <[hidden email]<mailto:[hidden email]>> 于2020年7月4日周六 上午12:13写道:
Hi.
I see somethings not quite right already.
Will start pr directly.

Xeno Amess <[hidden email]<mailto:[hidden email]>> 于2020年7月3日周五 下午4:20写道:
I might review it tonight if I have time...
Only if I have time.
Really busy these days...
Reply | Threaded
Open this post in threaded view
|

[CANCEL] [VOTE] Release Apache Commons Geometry (full distribution) 1.0-beta1 based on RC1

Matt Juntunen
This vote on rc1 is cancelled. I am going to start on rc2 but it may be a couple of days before I have time to complete it.
-Matt

________________________________
From: Matt Juntunen <[hidden email]>
Sent: Saturday, July 4, 2020 7:57 AM
To: Commons Developers List <[hidden email]>
Subject: Re: [VOTE] Release Apache Commons Geometry (full distribution) 1.0-beta1 based on RC1

Thank you for the thorough review, Xeno! I'm pulling those changes in now.
-Matt
________________________________
From: Xeno Amess <[hidden email]>
Sent: Friday, July 3, 2020 11:17 PM
To: Commons Developers List <[hidden email]>; Matt Juntunen <[hidden email]>
Subject: Re: [VOTE] Release Apache Commons Geometry (full distribution) 1.0-beta1 based on RC1

Hi.
I finished the review.
Suggestions details are listed on https://github.com/apache/commons-geometry/pulls
Notice that I am not familiar to this repo, nor a math expert, thus you should not fully trust my prs, you need to see them, and check if every change correct, because sometimes I make mistakes.

Xeno Amess <[hidden email]<mailto:[hidden email]>> 于2020年7月4日周六 上午10:09写道:
Hi.
I see in some places of the repo, two double fields are compared using ==.
Shouldn't we use Double.compare?
Or is it by design?

example places:

PolarCoordinates.equals
SphericalCoordinates.equals
Vector1D.angle
Vector2D.equals
Vector3D.equals
Vectors.isRealNonZero

Xeno Amess <[hidden email]<mailto:[hidden email]>> 于2020年7月4日周六 上午1:51写道:
Hi.
Please also consider about https://github.com/apache/commons-geometry/pull/85

I will perform more analyze opun this repo when you solve this two prs.

Xeno Amess <[hidden email]<mailto:[hidden email]>> 于2020年7月4日周六 上午1:01写道:
Hi.
Please check https://github.com/apache/commons-geometry/pull/84
In short, there be too many errors in javadoc.
(and this is the reason why I made https://github.com/apache/commons-parent/pull/7)

There still exist some other problems, but I'd prefer check them after this pr, as resolve conflict is boring.


Xeno Amess <[hidden email]<mailto:[hidden email]>> 于2020年7月4日周六 上午12:13写道:
Hi.
I see somethings not quite right already.
Will start pr directly.

Xeno Amess <[hidden email]<mailto:[hidden email]>> 于2020年7月3日周五 下午4:20写道:
I might review it tonight if I have time...
Only if I have time.
Really busy these days...
Reply | Threaded
Open this post in threaded view
|

[geometry] Inconsistent hashCode implementations

Alex Herbert
Xeno raised the use of Double.compare to be used to test for equality of floating point values instead of ==.

This should only be done if -0.0 and 0.0 are considered different. If this is the case then the accompanying hashCode value should not use Double.hashCode since when two objects are equal they must have the same hash code.

This is not the case for:

EpsilonDoublePrecisionContext
Vector1D
Vector2D
Vector3D
SphericalCoordinates
PolarCoordinates

When some of the numbers are 0.0 (or -0.0) as they use == for equality then their hash codes are different. In the cases of:

AffineTransformMatrix2D
AffineTransformMatrix3D
AxisAngleSequence

When the doubles are 1 ulp apart they are equal as they use Precision.equals for equality but they will have a different hash code. (Note that AffineTransformMatrix1D does not use Precision.equals but Double.compare == 0 so is fine.)

I think I have found all the cases of a broken hashCode but a search should be made for Double.hashCode and check all the hashCode and equals implementations are consistent.


If Precision.equals is to be used in the equals method then it will require some changes to have a correct hashCode implementation. There are 3 options:

1. Document the inconsistency

In this case the class should be documented as being inconsistent with the hashCode contract, something like:

<p>Note: This class has a hashCode that is inconsistent with equals.


2. Switch to binary equality

Provide a binary equality in equals and then a second method for equals using a precision context.


3. Smarter hashCode functions

For the case where 0.0 == -0.0 is to be used in equals then the hash code can be produced using:

Double.hashCode(0.0 - x);

Since:

+0.0 - +0.0 = +0.0
+0.0 - -0.0 = +0.0

All other non-Nan values will have the sign inverted for the argument to hashCode and NaN will remain NaN.

For the cases using Precision, IIUC the Precision.equals is always used with an allowed 1 ulp difference. So the following would work:

Long.hashCode(Double.doubleToLongBits(0.0 - x) >>> 2);

This should discard the least significant 2 bits thus any differences of 1 ulp will be discarded from the number. Note I’ve not tested this approach.

Alex

Reply | Threaded
Open this post in threaded view
|

Re: [geometry] Inconsistent hashCode implementations

Xeno Amess
> This is not the case for:
>
> EpsilonDoublePrecisionContext
> Vector1D
> Vector2D
> Vector3D
> SphericalCoordinates
> PolarCoordinates
Indeed.
I tried to analyze it but get to know you need to treat -0.0 equals 0.0
here, thus I don't think I can simply change it to Double.compare.

And yes, hashCode is why I suggest you consider about the migration. If two
objects are equals to each other while having different hashCode, that
would be weird.

Alex Herbert <[hidden email]> 于2020年7月4日周六 下午8:57写道:

> Xeno raised the use of Double.compare to be used to test for equality of
> floating point values instead of ==.
>
> This should only be done if -0.0 and 0.0 are considered different. If this
> is the case then the accompanying hashCode value should not use
> Double.hashCode since when two objects are equal they must have the same
> hash code.
>
> This is not the case for:
>
> EpsilonDoublePrecisionContext
> Vector1D
> Vector2D
> Vector3D
> SphericalCoordinates
> PolarCoordinates
>
> When some of the numbers are 0.0 (or -0.0) as they use == for equality
> then their hash codes are different. In the cases of:
>
> AffineTransformMatrix2D
> AffineTransformMatrix3D
> AxisAngleSequence
>
> When the doubles are 1 ulp apart they are equal as they use
> Precision.equals for equality but they will have a different hash code.
> (Note that AffineTransformMatrix1D does not use Precision.equals but
> Double.compare == 0 so is fine.)
>
> I think I have found all the cases of a broken hashCode but a search
> should be made for Double.hashCode and check all the hashCode and equals
> implementations are consistent.
>
>
> If Precision.equals is to be used in the equals method then it will
> require some changes to have a correct hashCode implementation. There are 3
> options:
>
> 1. Document the inconsistency
>
> In this case the class should be documented as being inconsistent with the
> hashCode contract, something like:
>
> <p>Note: This class has a hashCode that is inconsistent with equals.
>
>
> 2. Switch to binary equality
>
> Provide a binary equality in equals and then a second method for equals
> using a precision context.
>
>
> 3. Smarter hashCode functions
>
> For the case where 0.0 == -0.0 is to be used in equals then the hash code
> can be produced using:
>
> Double.hashCode(0.0 - x);
>
> Since:
>
> +0.0 - +0.0 = +0.0
> +0.0 - -0.0 = +0.0
>
> All other non-Nan values will have the sign inverted for the argument to
> hashCode and NaN will remain NaN.
>
> For the cases using Precision, IIUC the Precision.equals is always used
> with an allowed 1 ulp difference. So the following would work:
>
> Long.hashCode(Double.doubleToLongBits(0.0 - x) >>> 2);
>
> This should discard the least significant 2 bits thus any differences of 1
> ulp will be discarded from the number. Note I’ve not tested this approach.
>
> Alex
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [geometry] Inconsistent hashCode implementations

Matt Juntunen
I just added GEOMETRY-99 and a PR [1] for this. I decided to standardize on using Double.compare() in equals() and Double.hashCode() in hashCode(). I completely removed usage of Precision.equals(). Let me know if there are still issues here.

-Matt

[1] https://github.com/apache/commons-geometry/pull/93

________________________________
From: Xeno Amess <[hidden email]>
Sent: Saturday, July 4, 2020 10:52 AM
To: Commons Developers List <[hidden email]>
Subject: Re: [geometry] Inconsistent hashCode implementations

> This is not the case for:
>
> EpsilonDoublePrecisionContext
> Vector1D
> Vector2D
> Vector3D
> SphericalCoordinates
> PolarCoordinates
Indeed.
I tried to analyze it but get to know you need to treat -0.0 equals 0.0
here, thus I don't think I can simply change it to Double.compare.

And yes, hashCode is why I suggest you consider about the migration. If two
objects are equals to each other while having different hashCode, that
would be weird.

Alex Herbert <[hidden email]> 于2020年7月4日周六 下午8:57写道:

> Xeno raised the use of Double.compare to be used to test for equality of
> floating point values instead of ==.
>
> This should only be done if -0.0 and 0.0 are considered different. If this
> is the case then the accompanying hashCode value should not use
> Double.hashCode since when two objects are equal they must have the same
> hash code.
>
> This is not the case for:
>
> EpsilonDoublePrecisionContext
> Vector1D
> Vector2D
> Vector3D
> SphericalCoordinates
> PolarCoordinates
>
> When some of the numbers are 0.0 (or -0.0) as they use == for equality
> then their hash codes are different. In the cases of:
>
> AffineTransformMatrix2D
> AffineTransformMatrix3D
> AxisAngleSequence
>
> When the doubles are 1 ulp apart they are equal as they use
> Precision.equals for equality but they will have a different hash code.
> (Note that AffineTransformMatrix1D does not use Precision.equals but
> Double.compare == 0 so is fine.)
>
> I think I have found all the cases of a broken hashCode but a search
> should be made for Double.hashCode and check all the hashCode and equals
> implementations are consistent.
>
>
> If Precision.equals is to be used in the equals method then it will
> require some changes to have a correct hashCode implementation. There are 3
> options:
>
> 1. Document the inconsistency
>
> In this case the class should be documented as being inconsistent with the
> hashCode contract, something like:
>
> <p>Note: This class has a hashCode that is inconsistent with equals.
>
>
> 2. Switch to binary equality
>
> Provide a binary equality in equals and then a second method for equals
> using a precision context.
>
>
> 3. Smarter hashCode functions
>
> For the case where 0.0 == -0.0 is to be used in equals then the hash code
> can be produced using:
>
> Double.hashCode(0.0 - x);
>
> Since:
>
> +0.0 - +0.0 = +0.0
> +0.0 - -0.0 = +0.0
>
> All other non-Nan values will have the sign inverted for the argument to
> hashCode and NaN will remain NaN.
>
> For the cases using Precision, IIUC the Precision.equals is always used
> with an allowed 1 ulp difference. So the following would work:
>
> Long.hashCode(Double.doubleToLongBits(0.0 - x) >>> 2);
>
> This should discard the least significant 2 bits thus any differences of 1
> ulp will be discarded from the number. Note I’ve not tested this approach.
>
> Alex
>
>