Parallelize Volume Interpolator

[JaydenR2305]

Resolves #1728

Use Dask to parallelize the volume_averaged_interpolator method defined in the CartesianGrid object

[github-actions]

Thank you for submitting a pull request (PR) to PlasmaPy! ✨ The future of the project depends on contributors like you, so we deeply appreciate it! 🌱

Our contributor guide has information on:

• Getting ready to contribute and the code contribution workflow
• Using astropy.units and plasmapy.particles
• Coding tips and guidelines
• Writing documentation (in reStructuredText with numpydoc style docstrings).
• Writing tests using pytest
• Adding a changelog entry (needed except for minor changes)

The bottom of this page shows several checks that are run for every PR. Don't worry if something broke! We break stuff all the time. 😺 Click on "Details" to learn why a check didn't pass. Please also feel free to ask for help. We do that all the time as well. 🌸 You can find us in our chat room or weekly community meeting & office hours. Here are some tips:

• Try fixing CI / Python 3.11 test failures first.
• Most pre-commit.ci - pr failures can be automagically fixed by commenting pre-commit.ci autofix below, followed by a git pull to bring the changes back to your computer.
• If pre-commit.ci - pr says that a function is too long or complex, try breaking up that function into multiple short functions that each do one thing. See also these tips on writing clean scientific software.
• If the CI / Documentation check ends with a cryptic error message, check out our documentation troubleshooting guide.
• For a documentation preview, click on Details next to docs/readthedocs.org:plasmapy.

If this PR is marked as ready for review, someone should stop by to provide a code review and offer suggestions soon. ✅ If you don't get a review within a few days, please feel free to send us a reminder.

Please also use SI units within PlasmaPy, except when there is strong justification otherwise or in some examples.

We thank you once again!

[codecov]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (c5bdbda) 98.21% compared to head (8668833) 98.21%.
Report is 47 commits behind head on main.

❗ Current head 8668833 differs from pull request most recent head f6ceba8. Consider uploading reports for the commit f6ceba8 to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2226   +/-   ##
=======================================
  Coverage   98.21%   98.21%           
=======================================
  Files          98       98           
  Lines        8568     8584   +16     
=======================================
+ Hits         8415     8431   +16     
  Misses        153      153           

Files Changed	Coverage Δ
plasmapy/plasma/grids.py	99.35% <100.00%> (+0.02%)	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[pheuer]

Looks like a pretty clean start! Some additional stuff I'd like to see

1. Some tests that show that this is actually faster, and under what conditions. It may be we want to have running in parallel/dask be an option and raise a warning if people use it with very small numbers of particles or something? These don't need to be CI tests, just something you run and report the results in this PR thread so we have a record of it.

2. Apply the same method to the nearest neighbor interpolator so we have it in both cases.

3. I presume this is the case since all the tests seem to be passing, but just confirm that the interpolator is giving the same result as before. If you keep 'serial' as an option, you could write a simple test that ensures that they give the same answer.

[pheuer]

Even the private static method should have a docstring that defines what each of these variables is

[pheuer]

@JaydenR2305 This SE page looks relevant: https://stackoverflow.com/questions/53351060/dask-shared-memory-in-parallel-model

One comment suggests the joblib package, which I've used before (works pretty well)

Edit: and this link within: https://stackoverflow.com/questions/10721915/shared-memory-objects-in-multiprocessing

[JaydenR2305]

I have changed from our current implementation of the volume interpolation to SciPy's which has offered a roughly 50% speed improvement. I wonder if it would be beneficial to partition the grid using some sort of octree approach and then passing the respective particles along for parallel processing?

[pheuer]

I have changed from our current implementation of the volume interpolation to SciPy's which has offered a roughly 50% speed improvement.

Hm, I'm surprised its fast but I guess the RegularGridInterpolator with the linear method should be basically the same as this volume averaging scheme, so that's probably good? In that case I wonder if we should eventually change the keyword from 'volume_averaged' to 'linear' in the run() method. If this PR gets merged with this change, I would open an issue for that keyword change.

I wonder if it would be beneficial to partition the grid using some sort of octree approach and then passing the respective particles along for parallel processing?

Yes I think that's a good way to do it: the challenge is still how to share the grid across branches? Making copies would be the easiest, but could be memory prohibitive for large grids? In that case the ideal number of cores would depend both on the ncores available and on the amount of RAM available.