Skip to content

Conversation

w8r
Copy link

@w8r w8r commented Jun 21, 2024

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
Copy link
Member

Fil commented Jun 21, 2024

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
Copy link
Author

w8r commented Jun 21, 2024

@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