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

LUCENE-8973: XYRectangle2D should work on float space #865

Closed
wants to merge 15 commits into from

Conversation

iverase
Copy link
Contributor

@iverase iverase commented Sep 10, 2019

New implementation of XYRectangle 2D that works on the float space. Because the encoding is not lineal, working on the encoding space changes the spatial relationship between the bounding box and the other objects.

@iverase
Copy link
Contributor Author

iverase commented Sep 10, 2019

Note: Should XYRectangle holds the values as floats instead of doubles?

Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is the right fix, but I agree we should review usage of doubles and floats here. Ideally we'd use floats everywhere since this matches the accuracy we support, but I believe we use doubles in some places (Tessellator?) for the purpose of sharing code with the geo code.


/** Builds a Rectangle2D from rectangle */
public static XYRectangle2D create(XYRectangle rectangle) {
return new XYRectangle2D((float)rectangle.minX, (float)rectangle.maxX, (float)rectangle.minY, (float)rectangle.maxY);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is right, float casts will round to the nearest float while we should actually round towards zero so that queries are accurate in the encoded space?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if the encoding works as you said. I test the following:

  public void testEncoding() {
   double val = ShapeTestUtil.nextDouble();
   double castVal = (double)(float) val;
   double qVal = XYEncodingUtils.decode(XYEncodingUtils.encode(val));
   assertEquals(castVal, qVal, 0.0);
  }

This seems to hold true.

# Conflicts:
#	lucene/sandbox/src/java/org/apache/lucene/document/XYShapeBoundingBoxQuery.java
# Conflicts:
#	lucene/sandbox/src/java/org/apache/lucene/document/XYShapeBoundingBoxQuery.java
@iverase
Copy link
Contributor Author

iverase commented Oct 16, 2019

I have updated the PR so the XYRectangle2D is now a Component2D

@iverase
Copy link
Contributor Author

iverase commented Dec 11, 2019

Fixed in #872

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants