You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "timgrein (via GitHub)" <gi...@apache.org> on 2024/04/14 11:17:17 UTC

[PR] Use Float.compare instead of '==' in XYPoint and XYCircle [lucene]

timgrein opened a new pull request, #13301:
URL: https://github.com/apache/lucene/pull/13301

   ## Closes https://github.com/apache/lucene/issues/13292
   
   The following test
   
   `./gradlew test --tests TestXYPoint.testEqualsAndHashCode -Dtests.seed=3ABEFE4D876DD310 -Dtests.nightly=true -Dtests.locale=es-419 -Dtests.timezone=Asia/Ulaanbaatar -Dtests.asserts=true -Dtests.file.encoding=UTF-8`
   
   failed, because when using `==` to compare to floats `-0.0` and `0.0 `will be considered as equal while their hashcode is different. By using `Float.compare` this issue is resolved as `0.0` and `-0.0` are not treated as equal as their bit pattern is different:
   
   ```
   jshell> Float.floatToRawIntBits(0.0f)
   $6 ==> 0
   
   jshell> Float.floatToRawIntBits(-0.0f)
   $7 ==> -2147483648
   ```
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] Use Float.compare/Double.compare instead of '==' in geo classes [lucene]

Posted by "timgrein (via GitHub)" <gi...@apache.org>.
timgrein commented on PR #13301:
URL: https://github.com/apache/lucene/pull/13301#issuecomment-2056057821

   @easyice Adapted the PR with some changes I've described in the PR description 👍 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] Use Float.compare/Double.compare instead of '==' in geo classes [lucene]

Posted by "easyice (via GitHub)" <gi...@apache.org>.
easyice merged PR #13301:
URL: https://github.com/apache/lucene/pull/13301


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] Use Float.compare instead of '==' in XYPoint and XYCircle [lucene]

Posted by "easyice (via GitHub)" <gi...@apache.org>.
easyice commented on PR #13301:
URL: https://github.com/apache/lucene/pull/13301#issuecomment-2054287978

   Thanks for looking into this!  I found more  classes like  `Point`, `Circle`, `Rectangle2D` using `==` for  `equals`,  their hashcode will also different between `0.0` and `-0.0`, do they need to make similar changes?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] Use Float.compare/Double.compare instead of '==' in geo classes [lucene]

Posted by "easyice (via GitHub)" <gi...@apache.org>.
easyice commented on PR #13301:
URL: https://github.com/apache/lucene/pull/13301#issuecomment-2056595089

   Thanks for adding those tests! this change looks good to me.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] Use Float.compare instead of '==' in XYPoint and XYCircle [lucene]

Posted by "timgrein (via GitHub)" <gi...@apache.org>.
timgrein commented on PR #13301:
URL: https://github.com/apache/lucene/pull/13301#issuecomment-2055667053

   > Thanks for looking into this! I found more classes like `Point`, `Circle`, `Rectangle2D` using `==` for `equals`, their hashcode will also different between `0.0` and `-0.0`, do they need to make similar changes?
   
   Taking a look! :) Will adapt the PR accordingly


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


Re: [PR] Use Float.compare/Double.compare instead of '==' in geo classes [lucene]

Posted by "timgrein (via GitHub)" <gi...@apache.org>.
timgrein commented on PR #13301:
URL: https://github.com/apache/lucene/pull/13301#issuecomment-2056734019

   > Thanks for adding those tests! this change looks good to me.
   
   Thanks! Feel free to merge as I don't have write access to the repo :)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org