Replace stopclusteridx with geohash logic - #48
Conversation
|
Thank you for this! I have several questions: 1.) I don't understand Claude's comment regarding the fixed cell size and its problematic behavior on a dataset with high density variation. As I understand the code, the geohash precision is also chosen once globally, based on the 2.) I don't yet buy Claude's argument that the Web Mercator projection on insert/lookup here is problematic. Surely it is more expensive to allocate and build an 3.) It is clear that a hash-based approach is more memory-efficient in the sparse setting. But what about the dense setting? If every geohash-cell in the input bounding box has at least one value (which it has in the , isn't there a significant memory overhead in storing the hash strings explicitly? Have you done any tests in this direction, e.g. a memory usage comparison on the full Germany dataset? Also, the grid cell size is currently hard-set to 10,000 Web Mercator units (translates to rougly 10km x 10km around the equator). That means that even on a dataset covering the entire globe, we only have 4000² grid cells. Encoded as a 1D-array with 32 bit integer values, this uses only 64 MB of RAM, which is hardly problematic. Also note that with a 10km x 10km cellSize, the grid will be (nearly) densely packed for an input dataset like Germany. 4.) I also wonder how the performance looks: I would expect a string build/map lookup to be generally less efficient than a simple 2D-array lookup (we could even trivially flatten this into a 1D-array). 5.) Related: the |
Ich fand den Grid Cell Cluster Teil sehr schwer zu lesen und fragil, und habe mit Claude Code dann die Logik mit Geohash implementiert.
Ich kann nachvollziehen wenn du (@patrickbr) nicht die gesamte Cluster Logik ersetzen willst, aber ich dachte es könnte es lesbarer und "cleaner" machen.
Folgender Text (von Claude) ist die "Begründung" für Geohash vs den Grid Cell Ansatz:
Why a geohash-based index is cleaner than the old grid (
StopClusterIdx)The old approach builds a dense 2D array sized to the full bounding box of all stops, in projected web-mercator coordinates:
xWidth × yHeightcells get allocated up front (make([][]map[int]bool, idx.xWidth)), even for the vast majority of cells that will never hold a stop. A single outlier stop far from the rest blows upwidth/heightand therefore the whole grid.cellWidth/cellHeightare chosen once at construction and applied uniformly, so there's no good size for a feed that mixes a dense city center with sparse rural stops — too coarse wastes lookup time, too fine wastes memory.latLngToWebMerc, an extra transform and a place for distance/precision bugs to hide.GetNeighborsByLatLon(theswX/swYunderflow checks,min/maxcalls) is exactly the kind of code that's easy to get subtly wrong — and is unrelated to the actual indexing problem.A geohash-based index avoids all of this:
precisionForMeterspicks the right cell size for a given search radius, so the same index structure works whether you're searching for stops 5m apart or 5km apart, instead of being locked into one cell size at construction time.latLngToWebMercstep and no associated distortion to reason about away from the equator.geohashNeighbors: the cell plus its 8 surrounding cells) rather than a hand-computed cell range with manual clamping — fewer places for off-by-one or underflow bugs.Net effect: less code dedicated to bookkeeping (bounding box, projection, clamped ranges), and a data structure that scales with the data instead of with the map's geographic extent.