Skip to content

Fixed a precision problem with large values in packEnclose#215

Open
w8r wants to merge 2 commits into
d3:mainfrom
w8r:main
Open

Fixed a precision problem with large values in packEnclose#215
w8r wants to merge 2 commits into
d3:mainfrom
w8r:main

Conversation

@w8r

@w8r w8r commented Jun 21, 2024

Copy link
Copy Markdown

I have added a test showing how packEnclose is failing with the x,y values above 1e5. It runs out of precision and throws an Error at line 46 during the front propagation.

I've added translation of the circles to encloseBasis3, so that the overall values are smaller.

The time complexity is intact. I had to update the expected result in the edge case test (precision change).

Thank you for the wonderful algorithm implementation!

Example

const circles = [
    { "x": 531214.7271532917, "y": 360738.8204573899, "r": 10 },
    { "x": 531242.0429781883, "y": 360764.87727581244, "r": 10 },
    { "x": 531239.7927335791, "y": 360716.54336245544, "r": 10 }
  ];
  assert.doesNotThrow(() => packEnclose(circles));

@Fil

Fil commented Jun 21, 2024

Copy link
Copy Markdown
Member

Looking good! Note that these should be named large values rather than big numbers (because of precedent).

@w8r w8r changed the title Fixed a problem with big numbers in packEnclose Fixed a problem with large values in packEnclose Jun 21, 2024
@w8r

w8r commented Jun 21, 2024

Copy link
Copy Markdown
Author

@Fil I changed the wording

@Fil Fil requested a review from mbostock June 21, 2024 14:25
@w8r w8r changed the title Fixed a problem with large values in packEnclose Fixed a precision problem with large values in packEnclose Aug 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants