Add support for volumetric source spaces#15
Conversation
- select_vertices_in_sensor_range, restrict_forward_to_vertices, restrict_src_to_vertices, and restrict_forward_to_sensor_range now work with volumetric source spaces. - added a simple unittest to verify the above
There was a problem hiding this comment.
Wonderful to see work on volume source spaces! I'm happy to add this.
However, some of this code doesn't really spark joy. My issue is with the pattern of constantly using an if-statement to check whether we have 1 or 2 hemispheres. This branching makes the code somewhat tedious to read and reason about. I would be happier if some of the branching was replaced by code that works on an arbitrary number of hemispheres, so the same code just works regardless of whether we area dealing with a surface or volume source space.
|
I agree on the code quality. This was something I quickly figured out on a
flight but I can try to think of a more elegant way to implement this now
that it works. So still WIP.
|
Co-authored-by: Marijn van Vliet <w.m.vanvliet@gmail.com>
Co-authored-by: Marijn van Vliet <w.m.vanvliet@gmail.com>
Co-authored-by: Marijn van Vliet <w.m.vanvliet@gmail.com>
|
This should now be better and possibly work with arbitrary number of source spaces. |
|
Much cleaner! |
wmvanvliet
left a comment
There was a problem hiding this comment.
Very nice work @ruuskas. Thanks!
|
Thanks @wmvanvliet! |
We haven't discussed this previously, but this came up when working with a student. Would you consider merging?
I have checked that all tests still run locally.