Qinglei/redistribute sym - #794
QingleiCao wants to merge 1 commit into
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
🟡 Changes recommended
The new Open MPI detection in CMake should verify mpiexec --version succeeded before enabling --bind-to none to avoid breaking MPI test invocations on non-Open-MPI launchers.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overviewAdds symmetric (triangular) 2D block-cyclic descriptor support to the redistribute path and updates the collections test suite to exercise it, while also adjusting MPI reshape tests to avoid core-binding-related timeouts.
Changes:
- Add sym-2dbc as a supported redistribute distribution in tests and CLI parsing.
- Extend redistribute internals to recognize parsec_matrix_sym_block_cyclic_type for column batching and “region is stored” validation.
- Adjust collections/reshape/input_single_copy MPI tests to disable Open MPI rank binding (to prevent timeouts when oversubscribing cores).
| File | Description |
|---|---|
| tests/collections/Testings.cmake | Adds sym-2dbc to redistribute test matrix and adds Open MPI binding workaround for reshape tests. |
| tests/collections/redistribute/testing_redistribute.c | Adds symmetric 2DBC descriptor initialization and triangular-region/tile-stored handling in the test driver. |
| tests/collections/redistribute/common.h | Introduces REDISTRIBUTE_DIST_SYM_2DBC enum value for test CLI/config. |
| tests/collections/redistribute/common.c | Adds sym-2dbc string mapping/parsing and updates usage/help text accordingly. |
| parsec/data_dist/matrix/redistribute/redistribute_wrapper.c | Updates warning text to include symmetric block-cyclic support. |
| parsec/data_dist/matrix/redistribute/redistribute_internal.h | Includes sym descriptor header and extends internal helpers to handle sym block-cyclic descriptors. |
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Sorry, something went wrong.
There was a problem hiding this comment.
🟡 Changes recommended
The new CMake execute_process() logic can drop the mpiexec --version output due to reusing the same variable for stdout/stderr, preventing the intended Open MPI binding workaround from activating.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Lite
Sorry, something went wrong.
There was a problem hiding this comment.
🟢 Approval recommended
The changes consistently add symmetric 2DBC support end-to-end (runtime checks plus expanded tests) without introducing obvious correctness or API-contract issues.
Review details- Files reviewed: 6/6 changed files
- Comments generated: 0 new
- Review effort level: Lite
Sorry, something went wrong.
Reviewers
At least 1 approving review is required to merge this pull request.
Add support in redistribute for sym_two_dim_rectangle_cyclic_t