Skip to content

Comments

MPI_Allgatherv in mpi_allgather function#186

Open
rohanbabbar04 wants to merge 2 commits intomainfrom
allgatherv
Open

MPI_Allgatherv in mpi_allgather function#186
rohanbabbar04 wants to merge 2 commits intomainfrom
allgatherv

Conversation

@rohanbabbar04
Copy link
Collaborator

Closes #169

  • Use MPI_AllGatherv in mpi_allgather.

@rohanbabbar04
Copy link
Collaborator Author

I made a change which was bugging me when I was implementing MPI_Allgatherv, since it works for contiguous arrays only.

In Fredholm, y1 after transpose() becomes a non-contiguous array.

y1 = (
                    ncp.matmul(x.transpose(0, 2, 1).conj(), self.G)
                    .transpose(0, 2, 1)
                    .conj()
                )

This can be simplified to y1 = ncp.matmul(self.G.transpose(0, 2, 1).conj(), x) which is contiguous as well.

@mrava87
Copy link
Contributor

mrava87 commented Feb 17, 2026

Thanks @rohanbabbar04

When I suggested we should investigate Allgatherv I did not foresee the issue with non-contiguous arrays.

What you propose in the Fredholm code is a bit problematic though. We have used that transpose patterns for a real reason, G is usually much bigger than x, so by transposing twice x (before and after G is applied) we do effectively much fewer operations than transposing G... we do the same in PyLops' original operator, so changing this would require some proper benchmarking to do if the combo of changes you suggest is actually beneficial 😉

@rohanbabbar04
Copy link
Collaborator Author

rohanbabbar04 commented Feb 18, 2026

Ah!, Yes in that case we should keep it as before, because when G>>x then G transpose and conj will put pressure on memory.

I also think we should keep both Allgather and Allgatherv, whenever the shapes are variable we use Allgatherv otherwise the preferred method should be Allgather.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use in AllGatherv in mpi_allgather

2 participants