Skip to content

Conversation

@JordiManyer
Copy link
Contributor

@JordiManyer JordiManyer commented Aug 13, 2025

The current default algorithm to discover non-symmetric communication graphs is based on a gather-scatter algorithm. This works great for small communicators, but becomes prohibitive for large problems. The other available algorithm works great, and greatly reduces the amount of memory needed on the master processor.

Although there is technically a way to choose which algorithm you are using, it is extremely tedious and you have to go very low-level to actually be able to change it.
The option I've been using is to overwrite the default_find_rcv_ids function on my packages, but this breaks precompilation and is nothing more than a hack.

Due to this, I propose adding a compile-time preference through the Julia-standard Preferences.jl package. This changes the default behaviour of PartitionedArrays and sets the other algorithm as default.

The changes have zero impact on performance, since everything is done at compile time by using @static if-else statements.

Let me know what you think @fverdugo

@fverdugo
Copy link
Collaborator

Hi @JordiManyer,

Is is going to generate a Preferences.Toml file if a user does nothing?

@JordiManyer
Copy link
Contributor Author

Hi @fverdugo , not at all. The local file only gets created if the defaults get changed, like for MPI.jl. Otherwise the package bahaviour is unchanged and the user does not notice.

@fverdugo
Copy link
Collaborator

Good!

Can you then fix the documentation? It is failing to build.

@JordiManyer
Copy link
Contributor Author

Yes, Ill do some final changes if you are happy with the overall concept. Some questions:

  • Any comments/preferences on the function names?

  • Do you want to keep this like it is in a separate file? Where do I put the documentation?

@fverdugo
Copy link
Collaborator

Hi @JordiManyer

The other option is changing the default method (1 line of code) if it has been well tested.

@JordiManyer
Copy link
Contributor Author

JordiManyer commented Sep 4, 2025

Hi @fverdugo I would not do that. I believe the new function is robust and has been tested well, but I would still keep the old one as default. The new method is not compatible with non-distributed arrays (this includes DebugArray), so we would be doing things differently in mpi vs debug modes.
I think the current implementation is perfectly suited for testing, and only becomes a problem for very high core counts. So I would rather keep the replicability of results between mpi and debug modes.

@fverdugo
Copy link
Collaborator

fverdugo commented Sep 4, 2025

OK. Then let us go with the preferences approach. The method can also be changed at runtime with the current API if needed, right? If so, it looks good to me.

@JordiManyer
Copy link
Contributor Author

JordiManyer commented Sep 4, 2025

The method can also be changed at runtime with the current API if needed, right? If so, it looks good to me.

Yes! you can always provide an alternative to default_rcv_ids, like you could before.

I have updated the documentation, hopefully everything is OK now. I've added a couple of docstrings to make everything a little more coherent and added it to the ExchangeGraph documentation block.
Also: the docstring for find_rcv_ids_ibarrier was appearing in the backends.md page, so I moved it with the others.

I would say this is ready for review and merge.

@fverdugo
Copy link
Collaborator

fverdugo commented Sep 4, 2025

Hi @JordiManyer If you update the Project.toml and CHANGELOG.md with a new patch version, I can then register it right away after the merge.

@fverdugo fverdugo merged commit 483e5f4 into PartitionedArrays:master Sep 4, 2025
7 checks passed
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.

2 participants