locode: Use quadtree to find continent #830

Closed
Nesterfifa wants to merge 6 commits from Nesterfifa/frostfs-node:782-use-quadtree into master
11 changed files with 74 additions and 26 deletions
Showing only changes of commit 7578fd46e5 - Show all commits

View file

@ -7,10 +7,11 @@
package control package control
import ( import (
protoreflect "google.golang.org/protobuf/reflect/protoreflect"
protoimpl "google.golang.org/protobuf/runtime/protoimpl"
reflect "reflect" reflect "reflect"
sync "sync" sync "sync"
protoreflect "google.golang.org/protobuf/reflect/protoreflect"
dstepanov-yadro marked this conversation as resolved Outdated

Please drop this changes. These are generated files, so there is no need for automatic formatting.

Please drop this changes. These are generated files, so there is no need for automatic formatting.
protoimpl "google.golang.org/protobuf/runtime/protoimpl"
) )
const ( const (

View file

@ -8,6 +8,7 @@ package control
import ( import (
context "context" context "context"
grpc "google.golang.org/grpc" grpc "google.golang.org/grpc"
codes "google.golang.org/grpc/codes" codes "google.golang.org/grpc/codes"
status "google.golang.org/grpc/status" status "google.golang.org/grpc/status"

View file

@ -7,10 +7,11 @@
package control package control
import ( import (
protoreflect "google.golang.org/protobuf/reflect/protoreflect"
protoimpl "google.golang.org/protobuf/runtime/protoimpl"
reflect "reflect" reflect "reflect"
sync "sync" sync "sync"
protoreflect "google.golang.org/protobuf/reflect/protoreflect"
protoimpl "google.golang.org/protobuf/runtime/protoimpl"
) )
const ( const (

View file

@ -7,10 +7,11 @@
package control package control
import ( import (
protoreflect "google.golang.org/protobuf/reflect/protoreflect"
protoimpl "google.golang.org/protobuf/runtime/protoimpl"
reflect "reflect" reflect "reflect"
sync "sync" sync "sync"
protoreflect "google.golang.org/protobuf/reflect/protoreflect"
protoimpl "google.golang.org/protobuf/runtime/protoimpl"
) )
const ( const (

View file

@ -8,6 +8,7 @@ package control
import ( import (
context "context" context "context"
grpc "google.golang.org/grpc" grpc "google.golang.org/grpc"
codes "google.golang.org/grpc/codes" codes "google.golang.org/grpc/codes"
status "google.golang.org/grpc/status" status "google.golang.org/grpc/status"

View file

@ -7,10 +7,11 @@
package control package control
import ( import (
protoreflect "google.golang.org/protobuf/reflect/protoreflect"
protoimpl "google.golang.org/protobuf/runtime/protoimpl"
reflect "reflect" reflect "reflect"
sync "sync" sync "sync"
protoreflect "google.golang.org/protobuf/reflect/protoreflect"
protoimpl "google.golang.org/protobuf/runtime/protoimpl"
) )
const ( const (

View file

@ -10,10 +10,11 @@
package tree package tree
import ( import (
protoreflect "google.golang.org/protobuf/reflect/protoreflect"
protoimpl "google.golang.org/protobuf/runtime/protoimpl"
reflect "reflect" reflect "reflect"
sync "sync" sync "sync"
protoreflect "google.golang.org/protobuf/reflect/protoreflect"
protoimpl "google.golang.org/protobuf/runtime/protoimpl"
) )
const ( const (

View file

@ -11,6 +11,7 @@ package tree
import ( import (
context "context" context "context"
grpc "google.golang.org/grpc" grpc "google.golang.org/grpc"
codes "google.golang.org/grpc/codes" codes "google.golang.org/grpc/codes"
status "google.golang.org/grpc/status" status "google.golang.org/grpc/status"

View file

@ -10,10 +10,11 @@
package tree package tree
import ( import (
protoreflect "google.golang.org/protobuf/reflect/protoreflect"
protoimpl "google.golang.org/protobuf/runtime/protoimpl"
reflect "reflect" reflect "reflect"
sync "sync" sync "sync"
protoreflect "google.golang.org/protobuf/reflect/protoreflect"
protoimpl "google.golang.org/protobuf/runtime/protoimpl"
) )
const ( const (

View file

@ -4,6 +4,8 @@ import (
"fmt" "fmt"
"os" "os"
"github.com/paulmach/orb/quadtree"
fyrchik marked this conversation as resolved Outdated

Please, attach all non-stdlib imports in a single group (see paulmach/orb below).

Please, attach all non-stdlib imports in a single group (see `paulmach/orb` below).

Fixed

Fixed
locodedb "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util/locode/db" locodedb "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/util/locode/db"
"github.com/paulmach/orb" "github.com/paulmach/orb"
"github.com/paulmach/orb/geojson" "github.com/paulmach/orb/geojson"
@ -36,22 +38,24 @@ func (db *DB) PointContinent(point *locodedb.Point) (*locodedb.Continent, error)
minDst float64 minDst float64
) )
for _, feature := range db.features { pointer := db.tree.Matching(planarPoint, func(p orb.Pointer) bool {
if multiPolygon, ok := feature.Geometry.(orb.MultiPolygon); ok { return planar.PolygonContains(
if planar.MultiPolygonContains(multiPolygon, planarPoint) { p.(*geojson.Feature).Geometry.(orb.Polygon),
planarPoint,
)
})
if pointer != nil {
continent = pointer.(*geojson.Feature).Properties.MustString(continentProperty)
}
if continent == "" {
for _, feature := range db.features {
distance := planar.DistanceFrom(feature.Geometry, planarPoint)
if minDst == 0 || minDst > distance {
minDst = distance
continent = feature.Properties.MustString(continentProperty) continent = feature.Properties.MustString(continentProperty)
break
} }
} else if polygon, ok := feature.Geometry.(orb.Polygon); ok {
if planar.PolygonContains(polygon, planarPoint) {
continent = feature.Properties.MustString(continentProperty)
break
}
}
distance := planar.DistanceFrom(feature.Geometry, planarPoint)
if minDst == 0 || minDst > distance {
minDst = distance
continent = feature.Properties.MustString(continentProperty)
} }
} }
@ -73,6 +77,38 @@ func (db *DB) init() error {
db.features = features.Features db.features = features.Features
err = db.buildQuadtree()
if err != nil {
return fmt.Errorf("could not build quadtree: %w", err)
}
return nil
}
func (db *DB) buildQuadtree() error {
db.tree = quadtree.New(orb.Bound{
Min: orb.Point{-180, -180},
Max: orb.Point{180, 180},
})
for _, feature := range db.features {
Is there a way to simplify this code? https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/cff78d77c22b788a398a958a9a9d1f02b43b6148/pkg/util/locode/db/continents/geojson/calls.go#L94-L110

Reduced the nesting depth to 2

Reduced the nesting depth to 2
if multiPolygon, ok := feature.Geometry.(orb.MultiPolygon); ok {
fyrchik marked this conversation as resolved
Review

What about var multiPolygon orb.MultiPolygon? make(_, 0) is usually never needed: nil acts good enough as the default value.

What about `var multiPolygon orb.MultiPolygon`? `make(_, 0)` is usually never needed: `nil` acts good enough as the default value.
Review

Fixed

Fixed
for _, polygon := range multiPolygon {
newFeature := geojson.NewFeature(polygon)
newFeature.Properties = feature.Properties.Clone()
err := db.tree.Add(newFeature)
if err != nil {
return err
}
}
} else if _, ok := feature.Geometry.(orb.Polygon); ok {
err := db.tree.Add(feature)
if err != nil {
return err
}
}
}
return nil return nil
} }

View file

@ -5,6 +5,7 @@ import (
"sync" "sync"
"github.com/paulmach/orb/geojson" "github.com/paulmach/orb/geojson"
"github.com/paulmach/orb/quadtree"
) )
// Prm groups the required parameters of the DB's constructor. // Prm groups the required parameters of the DB's constructor.
@ -31,6 +32,8 @@ type DB struct {
once sync.Once once sync.Once
features []*geojson.Feature features []*geojson.Feature
tree *quadtree.Quadtree
} }
const invalidPrmValFmt = "invalid parameter %s (%T):%v" const invalidPrmValFmt = "invalid parameter %s (%T):%v"