NEW: Enable multi-node processing

[xiaodong-yu]

Purpose

Leverage mpi4py to reconstruct images using multiple nodes.

Approach

We create a class to wrap the inter-node p2p and collective communications using mpi4py. We then split the data into chunks and distribute them to corresponding nodes. We utilize the functions in our class to realize inter-node communications during ptychography.

Pre-Merge Checklists

Submitter

• [ ] Write a helpfully descriptive pull request title.
• [ ] Organize changes into logically grouped commits with descriptive commit messages.
• [ ] Document all new functions.
• [ ] Write tests for new functions or explain why they are not needed.
• [ ] Build the documentation successfully
• [ ] Use yapf to format python code.

Reviewer

• [x] Actually read all of the code.
• [x] Run the new code yourself.
• [ ] Write a summary of the changes as you understand them.
• [ ] Thank the submitter.

[pep8speaks]

Hello @xiaodong-yu! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file src/tike/ptycho/ptycho.py:

Line 197:81: E501 line too long (86 > 80 characters)

Comment last updated at 2021-01-22 19:54:19 UTC

[xiaodong-yu]

The purpose of this pull request is to add MPI-based parallelism to pytchography object updates. The approach used is to add an additional object for map-reduce parallelism.

I don't like this approach because it adds a second object whose purpose is to manage parallelism, but we already have an object which manages parallelism. Since both the ThreadPool and MPIComm are implementing map-reduce parallelism, why not create a combined object which manages both threads and nodes together?

Adding MPI-based parallelism also requires a larger discussion about the expected movement of data between nodes. For example, is the data loaded from the disk by one node and then distributed to the others? Or is it read in parallel and then shuffled around into contiguous blocks.

If you would like, we can have a Bluejeans discussion involving also Tekin, to talk about this issue as well as some other problems.

[carterbox]

These are the things that we discussed in our Bluejeans Meeting:

• [x] Combine thread and process parallelism classes into single abstract class composition
• [x] Assume the data is pre-split between processes before calling ptycho.reconstruct
• [ ] Allow for data to be not only within a single view but also along views axis
• [ ] Add tools for splitting data at load time?

The parallelism behavior: If less processes than views; retain previous behavior where multiple views share a single process and one or more threads. If more processes than views; each view gets multiple processes and one or more threads.

[xiaodong-yu]

These are the things that we discussed in our Bluejeans Meeting:

• [ ] Combine thread and process parallelism classes into single abstract class composition
• [ ] Assume the data is pre-split between processes before calling ptycho.reconstruct
• [ ] Allow for data to be not only within a single view but also along views axis
• [ ] Add tools for splitting data at load time?

The parallelism behavior: If less processes than views; retain previous behavior where multiple views share a single process and one or more threads. If more processes than views; each view gets multiple processes and one or more threads.

The pull request is updated accordingly.

[xiaodong-yu]

I'm going to run some tests locally with mpi enabled, but also we need some mpi-enabled tests on the CI.

Do you mean we should run CI with mpiexec?

[xiaodong-yu]

There are still unresolved comments from previous reviews, and yes, let's run the relevant tests with mpiexec. For example, add an additional test call like:

mpiexec -n 2 pytest tests/test_ptycho.py -k cgrad

How can I test mpiexec with CI? Do I need to add a test case to test_ptycho.py?

[xiaodong-yu]

Thanks, @xiaodong-yu! Also, I noticed your documentation is getting better; keep it up!

Thanks, @carterbox!