RxD indexing bug? #3337
RxD indexing bug? #3337
Conversation
| @@ -431,7 +431,7 @@ def node_by_ijk(self, i, j, k): | |||
| index += (i * e._ny + j) * e._nz + k | |||
| else: | |||
| e = ecs._region | |||
| index += e._nz * e._ny * e._nz | |||
| index += e._nx * e._ny * e._nz | |||
There was a problem hiding this comment.
Seems like an obviously correct fix. But the incremental indexing over the ecs loop this is embedded in is not clear to me at all. E.g I don't really know what is going on in this loop but I would have expected to break out of the loop at the end of the ecs == s fragment. So I guess this needs the author of this code to chime in. Do the rxd tests cover this code?
There was a problem hiding this comment.
I think Michael is right. @Hjorthmedh could you tell us more about the context in which you stumbled across this?
There was a problem hiding this comment.
We were digging through the code to understand how to use the extracellular feature in RxD. The line updated above jumped out at us when we were looking at the code as wrong.
I do not know the full context of this code, but from what I can see it looks like you need to find the index into the array storing all the nodes. And that the species appear to be stored one after the other, in an array (ie to get to the next species you need to step forward nxnynz.
I agree @nrnhines, there should be a break statement in the if, so I updated the pull-request. Otherwise the loop will keep increasing the index with (nxnynz) after it matched the requested ecs? But someone else please verify that we are correct.
@ramcdougal : Do you have unit tests that cover this function, and do they deal with non-cubic volumes?
|
We found a potential RxD bug...
#3335
Here is a suggestion for a fix. @ramcdougal