-
Notifications
You must be signed in to change notification settings - Fork 187
Port S2Earth #151
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
Comments
One thing to consider with making Distance a type is that there are already
minDistance and maxDistance types and the concept of a distance type (a la
https://github.com/google/s2geometry/blob/master/src/s2/s2distance_target.h)
which may lead to some confusion.
…On Sat, Apr 12, 2025 at 10:57 AM Trevor Stone ***@***.***> wrote:
I'm interested in adding Earth distance and area measurements to Go. There
are some API considerations I'd like input on.
Units
C++ S2Earth has three sets of methods, taking or returning meters
(double), kilometers (double), and util::units::Meters. The util class
uses 32-bit floats, I think because it's part of a general-purpose physical
units library. I see that S2Earth uses a radius of 6,371,010 meters, which
is about 3 meters larger than the R2 value of 6,371,007.1809 meters reported
by Wikipedia <https://en.wikipedia.org/wiki/Earth_radius>. We should
match the C++ value for numerical stability between languages.
In Go we could create something like type Distance float64 and let that
type do conversion between meters, kilometers, feet, miles, and any other
useful measure, without the awkward precision loss from the C++ unit class.
I think the following code will inline effectively, but haven't verified
this hypothesis:
// Copyright 2025 Google LLC. All rights reserved.
const (Meter Distance = 1e0Kilometer = 1e3 * MeterCentimeter = 1e-2 * MeterMillimeter = 1e-3 * MeterFoot = 0.3048 * MeterMile = 1609.344 * MeterInch = 0.0254 * Meter
)
func Meters(m float64) Distance { return Distance(m) * Meter }func Kilometers(km float64) Distance { return Distance(km) * Kilometer }func Centimeters(cm float64) Distance { return Distance(cm) * Centimeter }func Millimeters(mm float64) Distance { return Distance(mm) * Millimeter }func Feet(ft float64) Distance { return Distance(ft) * Foot }func Miles(mi float64) Distance { return Distance(mi) * Mile }func Inches(in float64) Distance { return Distance(in) * Inch }
func (m Distance) Millimeters() float64 { return float64(m / Millimeter) }func (m Distance) Centimeters() float64 { return float64(m / Centimeter) }func (m Distance) Meters() float64 { return float64(m) }func (m Distance) Kilometers() float64 { return float64(m / Kilometer) }func (m Distance) Inches() float64 { return float64(m / Inch) }func (m Distance) Feet() float64 { return float64(m / Foot) }func (m Distance) Miles() float64 { return float64(m / Mile) }func (m Distance) String() string { return fmt.Sprintf("%0.3f meters", m) }
The constant values allow other code to declare constants by
multiplication, e.g. if dist < 5 * unit.Mile or radius = 6371010 *
unit.Meter or const Furlong = unit.Mile / 8. One can also do if
dist.Miles() < 5. The constructors are useful when working with a float64
variable, instead of saying if dist < unit.Distance(minDistMiles) *
unit.Mile one can say if dist < unit.Miles(minDistMiles).
Using a Distance type that compiles to a float64 would let us have a
single set of Earth functions, e.g. AngleToDistance(s1.Angle) Distance
and DistanceToAngle(Distance) s1.Angle.
Area could work similarly, with square meters as the base value. This
approach enables easy conversions like acres or hectares without cluttering
the Earth measurement functions with every possible unit.
Packages
I can think of a couple approaches to package structure for this.
- Option 1: units and conversion in earth package.
var a, b s2.Point = …dist := earth.AngleToDistance(a.Distance(b))if dist < 5 * earth.Kilometers { … }
- Option 2: units in a unit package, conversion in an earth package.
This makes units look a little less weird if you're using S2 to model a
sphere that's not Earth, e.g. Mars or a basketball.
var a, b s2.Point = …dist := earth.AngleToDistance(a.Distance(b))if dist < 5 * unit.Kilometers { … }
- Option 3: an Earth value in the s2 package. Units could be in s2 or
a unit package. (I think a separate package would be clearer, since
distances are measures of s1 values and one could also create measurement
conversions for r2.)
var a, b s2.Point = …dist := s2.Earth.AngleToDistance(a.Distance(b))if dist < 5 * unit.Kilometers { … }
I think this would entail declaring a type (maybe type Radius
unit.Distance) and methods on that type, with const Earth Radius = 6371010
This would allow users to do measurements on other spherical bodies, e.g. const
Mars s2.Radius = 3389500 or const Basketball s2.Radius = 29.5 * unit.Inch.
I don't know if this receiver setup would have implications for inlining.
—
Reply to this email directly, view it on GitHub
<#151>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACBHGBBYLR7M6ITZUCVYZYT2ZFHY5AVCNFSM6AAAAAB3AHKVE6VHI2DSMVQWIX3LMV43ASLTON2WKOZSHE4TANRRGAZTCMY>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
*flwyd* created an issue (golang/geo#151)
<#151>
I'm interested in adding Earth distance and area measurements to Go. There
are some API considerations I'd like input on.
Units
C++ S2Earth has three sets of methods, taking or returning meters
(double), kilometers (double), and util::units::Meters. The util class
uses 32-bit floats, I think because it's part of a general-purpose physical
units library. I see that S2Earth uses a radius of 6,371,010 meters, which
is about 3 meters larger than the R2 value of 6,371,007.1809 meters reported
by Wikipedia <https://en.wikipedia.org/wiki/Earth_radius>. We should
match the C++ value for numerical stability between languages.
In Go we could create something like type Distance float64 and let that
type do conversion between meters, kilometers, feet, miles, and any other
useful measure, without the awkward precision loss from the C++ unit class.
I think the following code will inline effectively, but haven't verified
this hypothesis:
// Copyright 2025 Google LLC. All rights reserved.
const (Meter Distance = 1e0Kilometer = 1e3 * MeterCentimeter = 1e-2 * MeterMillimeter = 1e-3 * MeterFoot = 0.3048 * MeterMile = 1609.344 * MeterInch = 0.0254 * Meter
)
func Meters(m float64) Distance { return Distance(m) * Meter }func Kilometers(km float64) Distance { return Distance(km) * Kilometer }func Centimeters(cm float64) Distance { return Distance(cm) * Centimeter }func Millimeters(mm float64) Distance { return Distance(mm) * Millimeter }func Feet(ft float64) Distance { return Distance(ft) * Foot }func Miles(mi float64) Distance { return Distance(mi) * Mile }func Inches(in float64) Distance { return Distance(in) * Inch }
func (m Distance) Millimeters() float64 { return float64(m / Millimeter) }func (m Distance) Centimeters() float64 { return float64(m / Centimeter) }func (m Distance) Meters() float64 { return float64(m) }func (m Distance) Kilometers() float64 { return float64(m / Kilometer) }func (m Distance) Inches() float64 { return float64(m / Inch) }func (m Distance) Feet() float64 { return float64(m / Foot) }func (m Distance) Miles() float64 { return float64(m / Mile) }func (m Distance) String() string { return fmt.Sprintf("%0.3f meters", m) }
The constant values allow other code to declare constants by
multiplication, e.g. if dist < 5 * unit.Mile or radius = 6371010 *
unit.Meter or const Furlong = unit.Mile / 8. One can also do if
dist.Miles() < 5. The constructors are useful when working with a float64
variable, instead of saying if dist < unit.Distance(minDistMiles) *
unit.Mile one can say if dist < unit.Miles(minDistMiles).
Using a Distance type that compiles to a float64 would let us have a
single set of Earth functions, e.g. AngleToDistance(s1.Angle) Distance
and DistanceToAngle(Distance) s1.Angle.
Area could work similarly, with square meters as the base value. This
approach enables easy conversions like acres or hectares without cluttering
the Earth measurement functions with every possible unit.
Packages
I can think of a couple approaches to package structure for this.
- Option 1: units and conversion in earth package.
var a, b s2.Point = …dist := earth.AngleToDistance(a.Distance(b))if dist < 5 * earth.Kilometers { … }
- Option 2: units in a unit package, conversion in an earth package.
This makes units look a little less weird if you're using S2 to model a
sphere that's not Earth, e.g. Mars or a basketball.
var a, b s2.Point = …dist := earth.AngleToDistance(a.Distance(b))if dist < 5 * unit.Kilometers { … }
- Option 3: an Earth value in the s2 package. Units could be in s2 or
a unit package. (I think a separate package would be clearer, since
distances are measures of s1 values and one could also create measurement
conversions for r2.)
var a, b s2.Point = …dist := s2.Earth.AngleToDistance(a.Distance(b))if dist < 5 * unit.Kilometers { … }
I think this would entail declaring a type (maybe type Radius
unit.Distance) and methods on that type, with const Earth Radius = 6371010
This would allow users to do measurements on other spherical bodies, e.g. const
Mars s2.Radius = 3389500 or const Basketball s2.Radius = 29.5 * unit.Inch.
I don't know if this receiver setup would have implications for inlining.
—
Reply to this email directly, view it on GitHub
<#151>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACBHGBBYLR7M6ITZUCVYZYT2ZFHY5AVCNFSM6AAAAAB3AHKVE6VHI2DSMVQWIX3LMV43ASLTON2WKOZSHE4TANRRGAZTCMY>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
A few years back as part of a side side project I had starting making a Units package internally (somewhere under //analysis/economics/... or //analysis/common/... search for units. ) that was working on doing a lot of those same things because with a lot of the weather related data we imported and processed there was a lot of boilerplate unit conversion there that I wanted to extract out and make more general and useful. |
I don't think we want to add a general-purpose units library to S2: there's no need for mass or charge etc., and S2 has so far stayed away from the time domain, meaning users need to model GNSS readings with more than what S2 provides. |
I don't really know why this is the case. It was written in 2004, so maybe it made more sense with 387 FP.
There just aren't that many significant digits on the source that was used. https://web.archive.org/web/20050811075950/http://ssd.jpl.nasa.gov/phys_props_earth.html
We can also change the values, but I'd consider this low ROI.
Do we really need to support all these weird units? It's probably useful to look at how |
My thought in proposing functions which only work with typed units is to avoid multiplicative functions: C++ and Java S2Earth have three methods for each conceptual operation, one for meters, one for kilometers, and one for typed units, and the callers sometimes do their own conversions from meters or kilometers rather than using the typed units. In my proposal we don't need
The Google-internal version of S2Earth.java has Regarding the set of units: Google internally has several hundred files across all languages which use S2Earth and reference miles, and a handful each for inches, centimeters, acres, and hectares. Kilometer / meter / centimeter seems like a useful trio of length units on Earth: kilometers for travel-scale distances, meters for human-scale distances, and centimeters for point-size distances (level 30 S2 cells have edges slightly shorter than a centimeter). Mile / foot / inch are the similar-scale units in the U.S. Most land area measurements I've seen in the U.S. are in either acres or square miles; acres (640/mi2) is more popular for land ownership, square miles more common for large natural features like lakes, islands, and geopolitical entities. I don't have a sense of the relative frequency of hectares and square kilometers in land statistics outside the U.S. Square meters and square feet are desirable for human-scale measurements. |
I'm interested in adding Earth distance and area measurements to Go. There are some API considerations I'd like input on.
Units
C++ S2Earth has three sets of methods, taking or returning meters (double), kilometers (double), and
util::units::Meters
. The util class uses 32-bit floats, I think because it's part of a general-purpose physical units library. I see that S2Earth uses a radius of 6,371,010 meters, which is about 3 meters larger than the R2 value of 6,371,007.1809 meters reported by Wikipedia. We should match the C++ value for numerical stability between languages.In Go we could create something like
type Distance float64
and let that type do conversion between meters, kilometers, feet, miles, and any other useful measure, without the awkward precision loss from the C++ unit class. I think the following code will inline effectively, but haven't verified this hypothesis:The constant values allow other code to declare constants by multiplication, e.g.
if dist < 5 * unit.Mile
orradius = 6371010 * unit.Meter
orconst Furlong = unit.Mile / 8
. One can also doif dist.Miles() < 5
. The constructors are useful when working with a float64 variable, instead of sayingif dist < unit.Distance(minDistMiles) * unit.Mile
one can sayif dist < unit.Miles(minDistMiles)
.Using a Distance type that compiles to a float64 would let us have a single set of Earth functions, e.g.
AngleToDistance(s1.Angle) Distance
andDistanceToAngle(Distance) s1.Angle
.Area could work similarly, with square meters as the base value. This approach enables easy conversions like acres or hectares without cluttering the Earth measurement functions with every possible unit.
Packages
I can think of a couple approaches to package structure for this.
earth
package.unit
package, conversion in anearth
package. This makes units look a little less weird if you're using S2 to model a sphere that's not Earth, e.g. Mars or a basketball.Earth
value in thes2
package. Units could be ins2
or aunit
package. (I think a separate package would be clearer, since distances are measures of s1 values and one could also create measurement conversions for r2.)I think this would entail declaring a type (maybe
type Radius unit.Distance
) and methods on that type, withconst Earth Radius = 6371010
This would allow users to do measurements on other spherical bodies, e.g.const Mars s2.Radius = 3389500
orconst Basketball s2.Radius = 29.5 * unit.Inch
. I don't know if this receiver setup would have implications for inlining.The text was updated successfully, but these errors were encountered: