Skip to content
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 89 additions & 11 deletions s2/cell_index.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ type cellIndexNode struct {
// newCellIndexNode returns a node with the appropriate default values.
func newCellIndexNode() cellIndexNode {
return cellIndexNode{
cellID: 0,
cellID: CellID(0),
label: cellIndexDoneContents,
parent: -1,
}
Expand All @@ -49,15 +49,52 @@ type rangeNode struct {
contents int32 // Contents of this node (an index within the cell tree).
}

type labels []int32

func (l *labels) Normalize() {
encountered := make(map[int32]struct{})

for i := range *l {
encountered[(*l)[i]] = struct{}{}
}

*l = (*l)[:0]
for key := range encountered {
*l = append(*l, key)
}
}

type cellVisitor func(cellID CellID, label int32) bool

// CellIndexIterator is an iterator that visits the entire set of indexed
// (CellID, label) pairs in an unspecified order.
type CellIndexIterator struct {
// TODO(roberts): Implement
nodes []cellIndexNode
pos int
}

// NewCellIndexIterator creates an iterator for the given CellIndex.
func NewCellIndexIterator(index *CellIndex) *CellIndexIterator {
return &CellIndexIterator{}
return &CellIndexIterator{
nodes: index.cellTree,
pos: 0,
}
}

func (c *CellIndexIterator) CellID() CellID {
return c.nodes[c.pos].cellID
}

func (c *CellIndexIterator) Label() int32 {
return c.nodes[c.pos].label
}

func (c *CellIndexIterator) Done() bool {
return c.pos == len(c.nodes)
}

func (c *CellIndexIterator) Next() {
c.pos++
}

// CellIndexRangeIterator is an iterator that seeks and iterates over a set of
Expand Down Expand Up @@ -202,7 +239,6 @@ func (c *CellIndexRangeIterator) Seek(target CellID) {

// Nonempty needs to find the next non-empty entry.
for c.nonEmpty && c.IsEmpty() && !c.Done() {
// c.Next()
c.pos++
}
}
Expand Down Expand Up @@ -246,7 +282,7 @@ type CellIndexContentsIterator struct {
func NewCellIndexContentsIterator(index *CellIndex) *CellIndexContentsIterator {
it := &CellIndexContentsIterator{
cellTree: index.cellTree,
prevStartID: 0,
prevStartID: CellID(0),
nodeCutoff: -1,
nextNodeCutoff: -1,
node: cellIndexNode{label: cellIndexDoneContents},
Expand All @@ -256,7 +292,7 @@ func NewCellIndexContentsIterator(index *CellIndex) *CellIndexContentsIterator {

// Clear clears all state with respect to which range(s) have been visited.
func (c *CellIndexContentsIterator) Clear() {
c.prevStartID = 0
c.prevStartID = CellID(0)
c.nodeCutoff = -1
c.nextNodeCutoff = -1
c.node.label = cellIndexDoneContents
Expand Down Expand Up @@ -482,7 +518,8 @@ func (c *CellIndex) Build() {
c.cellTree = append(c.cellTree, cellIndexNode{
cellID: deltas[i].cellID,
label: deltas[i].label,
parent: contents})
parent: contents,
})
contents = int32(len(c.cellTree) - 1)
} else if deltas[i].cellID == SentinelCellID {
contents = c.cellTree[contents].parent
Expand All @@ -492,7 +529,48 @@ func (c *CellIndex) Build() {
}
}

// TODO(roberts): Differences from C++
// IntersectingLabels
// VisitIntersectingCells
// CellIndexIterator
func (c *CellIndex) IntersectingLabels(target CellUnion) []int32 {
Comment thread
rubenpoppe marked this conversation as resolved.
var labels labels
c.VisitIntersectingCells(target, func(cellID CellID, label int32) bool {
labels = append(labels, label)
return true
})

labels.Normalize()
return labels
}

func (c *CellIndex) VisitIntersectingCells(target CellUnion, visitor cellVisitor) bool {
if len(target) == 0 {
return true
}

pos := 0
contents := NewCellIndexContentsIterator(c)
rangeIter := NewCellIndexRangeIterator(c)
rangeIter.Begin()

for ok := true; ok; ok = pos != len(target) {
if rangeIter.LimitID() <= target[pos].RangeMin() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In C++ this was if rangeIter.LimitID() <= rangeIter.RangeMin() { is there a reason it's not like that here?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In C++ it was range.limit_id() <= it->range_min() where it is an iterator derived from target. Because there's no iterator I manually iterate over the cell ids.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rangeIter.Seek(target[pos].RangeMin())
}

for ; rangeIter.StartID() <= target[pos].RangeMax(); rangeIter.Next() {
for contents.StartUnion(rangeIter); !contents.Done(); contents.Next() {
if !visitor(contents.CellID(), contents.Label()) {
return false
}
}
}

pos++
Comment thread
rubenpoppe marked this conversation as resolved.
if pos != len(target) && target[pos].RangeMax() < rangeIter.StartID() {
pos = target.lowerBound(pos+1, len(target), rangeIter.StartID())
if target[pos-1].RangeMax() >= rangeIter.StartID() {
pos--
}
}
}

return true
}
102 changes: 100 additions & 2 deletions s2/cell_index_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ func cellIndexNodesEqual(a, b []cellIndexNode) bool {
return b[i].less(b[j])
})
return reflect.DeepEqual(a, b)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parent doesn't need to be equal and checked. (See LabelledCell in C++)


}

// copyCellIndexNodes creates a copy of the nodes so that sorting and other tests
Expand Down Expand Up @@ -338,9 +337,108 @@ func TestCellIndexRandomCellUnions(t *testing.T) {
cellIndexQuadraticValidate(t, "Random Cell Unions", index, nil)
}

func TestCellIndexIntersectionOptimization(t *testing.T) {
type cellIndexTestInput struct {
cellID string
label int32
}
tests := []struct {
label string
have []cellIndexTestInput
}{
{
// Tests various corner cases for the binary search optimization in
// VisitIntersectingCells.
label: "Intersection Optimization",
have: []cellIndexTestInput{
{"1/001", 1},
{"1/333", 2},
{"2/00", 3},
{"2/0232", 4},
},
},
}

for _, test := range tests {
index := &CellIndex{}
for _, v := range test.have {
index.Add(CellIDFromString(v.cellID), v.label)
}
index.Build()
checkIntersection(t, test.label, makeCellUnion("1/010", "1/3"), index)
checkIntersection(t, test.label, makeCellUnion("2/010", "2/011", "2/02"), index)
}
}

func TestCellIndexIntersectionRandomCellUnions(t *testing.T) {
// Construct cell unions from random CellIDs at random levels. Note that
// because the cell level is chosen uniformly, there is a very high
// likelihood that the cell unions will overlap.
index := &CellIndex{}
for i := int32(0); i < 100; i++ {
index.AddCellUnion(randomCellUnion(10), i)
}
index.Build()
for i := 0; i < 200; i++ {
checkIntersection(t, "", randomCellUnion(10), index)
}
}

func TestCellIndexIntersectionSemiRandomCellUnions(t *testing.T) {
for i := 0; i < 200; i++ {
index := &CellIndex{}
id := CellIDFromString("1/0123012301230123")
var target CellUnion
for j := 0; j < 100; j++ {
switch {
case oneIn(10):
index.Add(id, int32(j))
case oneIn(4):
target = append(target, id)
case oneIn(2):
id = id.NextWrap()
case oneIn(6) && !id.isFace():
id = id.immediateParent()
case oneIn(6) && !id.IsLeaf():
id = id.ChildBegin()
}
}
target.Normalize()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The call to Normalize wasn't in the C++. Does removing this change the test outcomes?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing target.Normalize() fails the test. I can't remember why anymore (my bad, left this open for too long).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

index.Build()
checkIntersection(t, "", target, index)
}
}

func checkIntersection(t *testing.T, desc string, target CellUnion, index *CellIndex) {
var expected, actual []int32
for it := NewCellIndexIterator(index); !it.Done(); it.Next() {
if target.IntersectsCellID(it.CellID()) {
expected = append(expected, it.Label())
}
}

index.VisitIntersectingCells(target, func(cellID CellID, label int32) bool {
actual = append(actual, label)
return true
})

if !labelsEqual(actual, expected) {
t.Errorf("%s: labels not equal but should be. %v != %v", desc, actual, expected)
}
}

func labelsEqual(a, b []int32) bool {
sort.Slice(a, func(i, j int) bool {
return a[i] < a[j]
})
sort.Slice(b, func(i, j int) bool {
return b[i] < b[j]
})
return reflect.DeepEqual(a, b)
}

// TODO(roberts): Differences from C++
//
// Add remainder of TestCellIndexContentsIteratorSuppressesDuplicates
//
// additional Iterator related parts
// Intersections related
1 change: 1 addition & 0 deletions s2/s2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ func randomCellUnion(n int) CellUnion {
for i := 0; i < n; i++ {
cu = append(cu, randomCellID())
}
cu.Normalize()
return cu
}

Expand Down