thi-ng / umbrella

⛱ Broadly scoped ecosystem & mono-repository of 198 TypeScript projects (and ~175 examples) for general purpose, functional, data driven development
https://thi.ng
Apache License 2.0
3.31k stars 144 forks source link

[geom-accel] SpatialGrid2 hangs runtime when using `set` method #463

Closed snosenzo closed 3 months ago

snosenzo commented 3 months ago

Usage of grid.set method below causes runtime to become unresponsive for an unknown reason.

Repro project setup:

package.json:

{
  "name": "test-thing",
  "scripts": {
    "dev": "tsx test.ts"
  },
  "dependencies": {
    "@thi.ng/geom-accel": "^3.5.69",
    "tsx": "^4.7.2"
  }
}

test.ts:

import { SpatialGrid2 } from "@thi.ng/geom-accel";

console.log("begin");

const grid = new SpatialGrid2([0, 0], [2, 2], 0.01);
console.log("constructed");
grid.set([0.01, 0.01], { x: 0.01, y: 0.01 });
console.log("end");
console.log([...grid.values()]);

Output of the script only gets to constructed. The script does not error, only seems to hang.

postspectacular commented 3 months ago

Hi @snosenzo — the problem here is that the 3rd param given to the SpatialGrid2 constructor is supposed to be a resolution (not a cell size). If you wanted your grid to have cells of 0.01 size then you'd need to specify 200 here, like so:

const grid = new SpatialGrid2([0, 0], [2, 2], 200);

// or use different sizes per axis
const grid = new SpatialGrid2([0, 0], [2, 2], [200, 100]);

FWIW Too high grid resolutions might actually be leading to lower performance due to reduced cache locality. It's best to experiment & benchmark different settings...

snosenzo commented 3 months ago

@postspectacular Oh, that makes sense! Thanks for clarifying :) Should this be changed to error if it receives a value less than 1 for the resolution? Happy to open a PR for it if that would help

postspectacular commented 3 months ago

Yeah, that's a great idea! Also generally still need to add some docs for the different impls which probably would have avoided your issue too... mea culpa!