Skip to content
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

Temporary fix for MPI bug in Projection.get #573

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lkoelman
Copy link

This is a rather subtle bug. With MPI enabled it fails with error "File "projections.py", line 352, in get() : return values[0] : IndexError: list index out of range"

The problem is that if gather is not 'all', on line 352 the method tries to address the array which is empty on nodes with rank != 0. Another solution would be to put that statement inside the preceding if-block and merge that if block with the enclosing one, so an empty 'values' is returned on nodes with rank != 0.

This is a rather subtle bug. The problem is that if gather is not 'all', on line 352 the method tries to address the array which is empty on nodes with rank != 0. Another solution would be to put that statement inside the preceding if-block and merge that if block with the enclosing one, so an empty 'values' is returned on nodes with rank != 0.
@apdavison
Copy link
Member

Many thanks for spotting this problem.

Concerning the proposed solution, I would prefer not to change the public API where it can be avoided. In addition, using 'all' increases the network traffic, so will potentially slow down many simulations unnecessarily.

Please could you create an Issue with a minimal script that illustrates the problem?

@lkoelman
Copy link
Author

That seems sensible. I'll implement the other proposed fix then? This may take a while as I'm currently busy with a deadline though (ISEK 2018).

PS: thanks for all the good work on PyNN and Neo, finding it a delight to work with :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants