bugfix: handle problems acos arg > 1#33
Open
tjansson60 wants to merge 1 commit intoTaipanRex:masterfrom
Open
Conversation
While processing the shoreline set GSHHS_c_L1 with 742 shapes a problem was encountered where cos_value = 1.00000000000048 which meant that the acos raised an ValueError: math domain error and the graph could not be built. This patch checks if the arguments are very close to -1 or 1 and returns the expected values. If the values are not close to either of these values the ValueError is still raised. point_a = (112.96, -25.49) point_b = (45.00, -25.49) point_c = (45.00, -25.49) a = 6.938890000001394e-07 b = 4618.002849783456 c = 4618.11606498842 cos_value = 1.00000000000048 T = 10000000000000 T2 = 10000000000000.0
Owner
|
I'll need to performance test this. Check out the discussion on angle2() in #28, by setting COLIN_TOLERANCE to a lower value, like 8, it should solve this problem without any performance hit. Its not explained well in the code and I'm working on better naming the COLIN_TOLERANCE, T1, T2 variables and also allowing the user to set COLIN_TOLERANCE through the API (see #29). |
Contributor
Author
TaipanRex
added a commit
that referenced
this pull request
May 30, 2018
Still seeing some errors so reduced the COLIN_TOLERANCE from 13 to 10. This solves the specific example given in #33 and I also mentioned in #28 that I would reduce the tolerance to better avoid these types of errors. @tjansson60 has a suggested solution that does not involve truncating the cos_value, I need to performance test this first. See: #33, #28, #19
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While processing the shoreline set GSHHS_c_L1 with 742 shapes a
problem was encountered where cos_value = 1.00000000000048 which meant
that the acos raised an ValueError: math domain error and the graph
could not be built.
This patch checks if the arguments are very close to -1 or 1 and returns
the expected values. If the values are not close to either of these
values the ValueError is still raised.
Examples of values in angle2 when problem was first encountered:
point_a = (112.96, -25.49)
point_b = (45.00, -25.49)
point_c = (45.00, -25.49)
a = 6.938890000001394e-07
b = 4618.002849783456
c = 4618.11606498842
cos_value = 1.00000000000048
T = 10000000000000
T2 = 10000000000000.0