-
Notifications
You must be signed in to change notification settings - Fork 364
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use shape as argument instead of columns/rows/depth and update topology examples to NEST3 #1287
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this looks so much better! I just have a tiny comment, but otherwise I definitely approve.
# generate list of 12 (x,y) pairs | ||
pos = [[random.uniform(-0.75, 0.75), random.uniform(-0.5, 0.5)] | ||
for j in range(12)] | ||
pos = nest.spatial.free([[random.uniform(-0.75, 0.75), random.uniform(-0.5, 0.5)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe use nest.random.uniform here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
numpy.random.uniform?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Silmathoron with the new NEST-3 we have made random distributions from NEST accessible from PyNEST, and seeing as this is an example I think we should use the NEST version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem here is that we want different distributions for different dimensions, which currently can't be done when passing nest.random
functions to nest.spatial.free()
. But it shouldn't be difficult to add this, maybe after this PR is merged, then we can update this example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yesterday, I asked @hakonsbm about this and he recommended to use numpy.random.uniform in this case because with nest.random.uniform it is apparently not possible, yet, to define different min and max for the two dimensions. If there is a better solution, please let me know and I will adjust it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, passing a nest.random
function to nest.spatial.free()
will currently use the same distribution in all dimensions. But it shouldn't be difficult to add the possibility to have different distributions for different dimensions. Maybe after this PR is merged, then we can update this example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# generate list of 12 (x,y) pairs | ||
pos = [[random.uniform(-0.75, 0.75), random.uniform(-0.5, 0.5)] | ||
for j in range(12)] | ||
pos = nest.spatial.free([[random.uniform(-0.75, 0.75), random.uniform(-0.5, 0.5)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem here is that we want different distributions for different dimensions, which currently can't be done when passing nest.random
functions to nest.spatial.free()
. But it shouldn't be difficult to add this, maybe after this PR is merged, then we can update this example.
This PR replaces the arguments
columns
/rows
/depth
used previously for creating grid layers with a single array namedshape
.shape
can be of length 2 or 3 for 2D or 3D layers, respectively. The grid mask and its anchor are also adapted.Topology examples are now updated to NEST3 and moved to pynest/examples/spatial.