Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make MPI tags deterministic #462

Merged
merged 4 commits into from
Nov 7, 2023
Merged

make MPI tags deterministic #462

merged 4 commits into from
Nov 7, 2023

Conversation

matthiasdiener
Copy link
Collaborator

@matthiasdiener matthiasdiener commented Oct 10, 2023

This requires tags to be sortable. The only other option I can see is to use some kind of ordered set type.

@matthiasdiener matthiasdiener self-assigned this Oct 10, 2023
@matthiasdiener matthiasdiener marked this pull request as ready for review October 18, 2023 23:22
@matthiasdiener matthiasdiener changed the title make MPI tag deterministic make MPI tags deterministic Oct 19, 2023
@@ -99,7 +99,7 @@ def set_union(
next_tag = base_tag
assert isinstance(all_tags, frozenset)

for sym_tag in all_tags:
for sym_tag in sorted(all_tags):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are they necessarily ordered? What are we sorting by?

@inducer inducer force-pushed the deterministic-mpi_tag branch from 7e175bc to eac1bd7 Compare November 7, 2023 15:56
@inducer
Copy link
Owner

inducer commented Nov 7, 2023

Pushed some minor stuff, LGTM. Thanks!

@inducer inducer enabled auto-merge (squash) November 7, 2023 15:56
@inducer inducer merged commit 691d38a into main Nov 7, 2023
9 checks passed
@inducer inducer deleted the deterministic-mpi_tag branch November 7, 2023 16:16
inducer added a commit that referenced this pull request Nov 13, 2023
Co-authored-by: Andreas Kloeckner <inform@tiker.net>
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