nest-simulator icon indicating copy to clipboard operation
nest-simulator copied to clipboard

Inconsistencies for corner cases of creating spatial layers

Open heplesser opened this issue 3 years ago • 4 comments

I noticed some corner cases when creating spatial layers, which call for improvement.

Confusing error message in case of negative extent

When creating a layer with negative extent, the error message should report the illegal extent value (>0 required), not a node outside the layer:

In [40]: nest.Create('iaf_psc_alpha', positions=nest.spatial.free([[1,2], [3,4]], extent=[-5, 5]))
---------------------------------------------------------------------------
NESTErrors.BadProperty                    Traceback (most recent call last)
...
NESTErrors.BadProperty: BadProperty in SLI function CreateLayer_D_D: Node position outside of layer

Identical positions lead to extent 0

I think all dimensions of the extent should always be strictly larger than 0, so the following should trigger an error:

l3 = nest.Create('iaf_psc_alpha', positions=nest.spatial.free([[1,2], [1,2]]))

In [42]: l3.spatial
Out[42]: 
{'center': (1.0, 2.0),
 'edge_wrap': False,
 'extent': (0.0, 0.0),
 'network_size': 2,
 'positions': ((1.0, 2.0), (1.0, 2.0))}

Incorrect extent

In the following case, according to my understanding, the extent should be (2.0, 2.1), but it is (2.0, 3.0); the center is correct:

In [48]: l2 = nest.Create('iaf_psc_alpha', positions=nest.spatial.free([[1,2], [3, 4.1]]))

In [49]: l2.spatial
Out[49]: 
{'center': (2.0, 3.05),
 'edge_wrap': False,
 'extent': (2.0, 3.0),
 'network_size': 2,
 'positions': ((1.0, 2.0), (3.0, 4.1))}

Outdated documentation

The documentation for spatial.free, especially num_dimensions is no longer up to date after #2459, because a single position now strictly requires extent:

https://github.com/nest/nest-simulator/blob/ffcbe0572931b4d8350205bcc9da17a783111763/pynest/nest/spatial/hl_api_spatial.py#L218-L220

heplesser avatar Sep 08 '22 11:09 heplesser

In the following case, according to my understanding, the extent should be (2.0, 2.1), but it is (2.0, 3.0)

@heplesser I may be wrong, but I don't think it's stated anywhere that the extent has to be exactly the distance between the "lower left" node and the "upper right" node. Rounding up the extent avoids round-off errors where a node erroneously may appear outside of the layer.

Furthermore, rounding up the extent makes it behave as expected when creating randomly placed positions. Take for example creating nodes like this:

nest.Create('iaf_psc_alpha', 
            num_nodes, 
            positions=nest.spatial.free(nest.random.uniform(-r, r), num_dimensions=2))

Rounding up the extent will put it at (2*r, 2*r), as expected (see #2459).

hakonsbm avatar Sep 15 '22 07:09 hakonsbm

@hakonsbm I think I overlooked the rounding in #2459, but I think it problematic, because it rounds to integers and thus presumes a specific scale of the extent, numbers close to 1. The result can be highly unpredictable if we have large r and small num_nodes in the example above. Here are center and extent for five runs with r=10 and num_nodes=5:

Center Extent
7.52, -0.94 3, 13
-1.60, -1.70 16, 16
-2.14, 0.27 8, 12
0.58, 1.49 16, 17
-0.75, -3.33 18, 13

While this is a rather extreme case, I wonder if we should not require center and extent explicitly in the future, maybe with defaults (0, 0) for the center and (1, 1) for the extent. We could also as additional way of specifying center and extent introduce a bounding-box parameter, a tuple with coordinates of lower left and upper right point.

Should we discuss this in the next open developer VC?

heplesser avatar Sep 15 '22 10:09 heplesser

Sure, let's discuss it in the open developer VC 26 September.

hakonsbm avatar Sep 15 '22 11:09 hakonsbm

Issue automatically marked stale!

github-actions[bot] avatar Nov 15 '22 08:11 github-actions[bot]