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

optimize ring area calculation #1387

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

pirxpilot
Copy link

  • unwind special (n - 2 and n - 1) cases to simplify the loop
  • keep track lower, middle, and upper value in radians so that we don't need to recalculate them while traversing the ring

bench before: polygon x 4,280,634 ops/sec ±1.53% (89 runs sampled)
bench after: polygon x 5,810,796 ops/sec ±1.09% (87 runs sampled)

pirxpilot and others added 2 commits May 14, 2018 11:12
- unwind special (n - 2 and n - 1) cases to simplify the loop
- keep track lower, middle, and upper value in radians so that we don't
  need to recalculate them while traversing the ring

bench before:
polygon x 4,280,634 ops/sec ±1.53% (89 runs sampled)

bench after:
polygon x 5,810,796 ops/sec ±1.09% (87 runs sampled)
@rowanwins
Copy link
Member

Hi @pirxpilot

Sorry its taken a long time for someone to review this.

Looking at the Travis test result the change to this module is causing issues in @turf/isobands. I suspect we're seeing a change to decimal precision. Because this module has only 1 test itself and it's rounding the result to the nearest integer it's not exactly a super-thorough test.

I'll need to do a bit more in-depth testing on isobands to make sure it's all working out as expected.

AminoffZ added a commit to AminoffZ/turf that referenced this pull request Oct 22, 2023
@AminoffZ AminoffZ mentioned this pull request Oct 22, 2023
4 tasks
twelch pushed a commit that referenced this pull request Nov 14, 2023
* Merge pull request #1387 from pirxpilot:area

* Fix optimized ringArea

* Regen nearest-neighbor-analysis

* Add rest of out tests

* update turf-area benchmark
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants