Simplify dLambda handling in area calculation#294
Conversation
Refactor spherical excess calculation to simplify dLambda handling.
Fil
left a comment
There was a problem hiding this comment.
Great remark. I started digging into the code history to see when that change in the formula appeared:
(the tests never came, so I don't know if the issue from Chrome 33 is still relevant today)
|
Testing various browsers at random I didn't find any case of dissymmetry |
|
Thanks for digging in. I tried 4 of the polygons mentioned in d3/d3#1753, and I'm getting the expected area (or at least the sign) for each of them when using my H3 algorithm in C/Python, so I don't think we're suffering from the same issue. This is super helpful to know about tho, in case the issue does come up---maybe in the h3-js bindings? Although, it sounds like this may no longer be an issue in Chrome either... If the bug is resolved in Chrome, I wonder if you guys get any performance benefit from dropping the extra computation? But I'll leave that to you, since I was mostly interested in our H3 use case. Thanks again for the help! For future reference, here were the polygons I tested. Note, I had to correct the winding order in polygon 3 of 4 (from what was given in d3/d3#1753): And I get the following steradian areas: |
Hi! I was looking at your code here because I'm currently trying to improve our own spherical area calculations in https://github.com/uber/h3, and this has been a super helpful reference.
If I'm not mistaken, I think you can slightly simplify the calculation you have here. Since
sin(-x) = -sin(x)andcos(-x) = cos(x), this change should produce exactly the same output. Or, I'm wrong and I learn something, which will be helpful for our use case :)