Basic Planar fns, Simplifier interface #6

gdey · 2018-05-02T17:38:13Z

Added some handy planar functions, and defined an Interface for
Simplifers. This way we can add and change out simplifiers easily.

Added some handy planar functions, and defined an Interface for Simplifers. This way we can add and change out simplifiers easily.

gdey · 2018-05-02T17:39:04Z

I know there aren't enough tests on this currently, but was not expecting to put it up quite yet. But it should help you @jasonsurratt.

coveralls · 2018-05-02T17:46:34Z

Pull Request Test Coverage Report for Build 49

13 of 91 (14.29%) changed or added relevant lines in 7 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-1.9%) to 77.587%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
encoding/wkb/wkb.go	0	1	0.0%
geom.go	0	1	0.0%
errors.go	0	3	0.0%
planar/planar.go	11	24	45.83%
planar/simplify.go	0	60	0.0%

Totals
Change from base Build 37:	-1.9%
Covered Lines:	2354
Relevant Lines:	3034

💛 - Coveralls

jasonsurratt

Please have a look at my questions/comments. I may not understand the intent in some cases.

jasonsurratt · 2018-05-02T18:38:32Z

planar/planar.go

+	m1, _, _ := Slope([2][2]float64{pt2, pt1})
+	m2, _, _ := Slope([2][2]float64{pt3, pt1})
+	// 6 digits of prec
+	d, _ := new(big.Float).SetPrec(6*4).Sub(big.NewFloat(m2), big.NewFloat(m1)).Float64()


Why are you limiting this to 6 digits of precision when float64 supports ~15?

Also, this seems to be returning the difference in slope between the three lines, not the angle between the three lines. I think if you want angle that would be calling atan2.

The SetPrec is really me guessing. I think I will just remove it, when I have a better answer we can add it.

jasonsurratt · 2018-05-02T18:48:19Z

planar/planar.go

+	}
+
+	// 4 digits of precision
+	d, _ := new(big.Float).SetPrec(16).Mul(big.NewFloat(dist), semimajor).Float64()


4 digits of precision, or 16? What does this operation achieve over dist * semimajor?

jasonsurratt · 2018-05-02T19:05:11Z

planar/planar.go

+				math.Cos(rpt2x)*
+				(math.Pow(math.Sin(disty/2), 2)),
+		),
+	)


FYI This is the hoot haversine implementation I mentioned previously. Looks like I was getting about 9cm precision using float64 with Haversine (likely plenty for your needs).

There are differences in the implementation, but there are a bunch of ways to get to the same answer and I'm sure your tests will reveal it if there are any problems.

I don't think I can use hootenanny's implementation as it's GPL'd.

jasonsurratt · 2018-05-02T19:05:16Z

planar/planar.go

+	dist := 2 * math.Asin(
+		math.Sqrt(
+
+			(math.Pow(math.Sin(distx/2), 2)+math.Cos(rpt1x))*


You may want to replace math.Pow(x, 2) with x * x. Likely quite a bit faster.

Good point.

jasonsurratt · 2018-05-02T19:09:21Z

planar/planar.go

+func (hs Haversine) PerpendicularDistance(line [2][2]float64, point [2]float64) float64 {
+	dist := hs.Distance(line[0], point)
+	angle := Angle3Pts(line[0], line[1], point)
+	return math.Abs(dist * math.Sin(angle))


If I'm reading this right, this seems to be mixing planar operations (Angle3Pts) and spherical operations (hs.Distance). Based on the comment, I think you're looking for Cross-track distance.

Yes, I'm not calculating the Angle3Pts correctly. I should be using a radian_slop function instead of a euclidian slop function. Good catch.

jasonsurratt · 2018-05-02T19:10:55Z

planar/planar_test.go

+			line:    [2][2]float64{{0, 0}, {10, 10}},
+			m:       1,
+			b:       0,
+			defined: true,


With all the test framework in place, you'll want to add some more tests. For instance, this won't catch a case where x/y get swapped.

Yeah, I don't have very good tests in there right now. I was still in the process of converting over the code, and did not have time to add all the test cases. This was just to get the framework for the test in place, to make it easy to add more.

jasonsurratt · 2018-05-02T19:14:25Z

planar/planar.go

+	deltaX := line[1][0] - line[0][0]
+	deltaY := line[1][1] - line[0][1]
+	denom := math.Abs((deltaY * point[0]) - (deltaX * point[1]) + (line[1][0] * line[0][1]) - (line[1][1] * line[0][0]))
+	num := math.Sqrt(math.Pow(deltaY, 2) + math.Pow(deltaX, 2))


Why math.Pow(deltaY, 2) instead of deltaY * deltaY?

jasonsurratt · 2018-05-02T19:37:52Z

planar/planar.go

+	if num == 0 {
+		return 0
+	}
+	return denom / num


I think you swapped the naming of num & denom.

jasonsurratt · 2018-05-02T19:39:02Z

planar/planar.go

+
+type PointLineDistanceFunc func(line [2][2]float64, point [2]float64) float64
+
+// PerpendicularDistance  provides the distance between a line and a point in Euclidean space.


Please add a reference to: https://en.wikipedia.org/wiki/Distance_from_a_point_to_a_line

"Line defined by two points" section

added NewTileLatLon funcitonnality added an iterator function RangeFamilyAt to Tile Added tests and bug fixes for lat/lon -> tile conversion Added reference to the formula for PerpendicularDisance. Added references to docs used for algos. Removing spherical functions as they are not correct.

Basic Planar fns, Simplifier interface

6449bd7

Added some handy planar functions, and defined an Interface for Simplifers. This way we can add and change out simplifiers easily.

gdey requested review from ARolek and jasonsurratt May 2, 2018 17:38

jasonsurratt suggested changes May 2, 2018

View reviewed changes

gdey force-pushed the planar_fns branch from 67f9d31 to 0da3a09 Compare May 3, 2018 20:56

gdey added 2 commits May 21, 2018 10:27

Vendored rtreego into internal.

218bf7e

gdey force-pushed the planar_fns branch from 5c77323 to 83b97c7 Compare May 21, 2018 17:29

Merge branch 'master' into planar_fns

269f94d

gdey force-pushed the planar_fns branch from 78a31a0 to 269f94d Compare May 21, 2018 17:35

gdey merged commit 3d919e2 into master May 21, 2018

gdey deleted the planar_fns branch May 21, 2018 18:03


		type PointLineDistanceFunc func(line [2][2]float64, point [2]float64) float64

		// PerpendicularDistance provides the distance between a line and a point in Euclidean space.

Uh oh!

Basic Planar fns, Simplifier interface #6

Basic Planar fns, Simplifier interface #6

Uh oh!

Conversation

gdey commented May 2, 2018

Uh oh!

gdey commented May 2, 2018

Uh oh!

coveralls commented May 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 49

💛 - Coveralls

Uh oh!

jasonsurratt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coveralls commented May 2, 2018 •

edited

Loading