Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently the
get_array_binary_mask
andcombine_sensor_data
of the kWaveArray are pretty slow for a large number of array elements.They would benefit from
This (draft) PR implements a few ideas for the refactoring part.
get_array_binary_mask:
off_grid_points
all at once, which saves many calls tooff_grid_points
and avoids or'ing the final maskcombine_sensor_data
added options to input the sensor_mask, so it doesn't have to be calculated a second time
added an option to return the grid_weights sparsely. Currently the weights are collected in an array, then
matlab_find
is used to find the non-zero voxels. Insteadoff_grid_points
can compute the grid_weights in a collections.Counter, so that we don't have to additionally find them in a large array.additional idea: make
off_grid_points
return a function that can quickly return the weights of an element without all the other stuff that happens inoff_grid_points
. This could be useful in the element loop ofcombine_sensor_data
, where we have to compute the weights of each element individually.Regarding parallelization: I think joblib would probably be a good choice, but I don't have a lot of experience with that.