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

Fix interpolate! for MPI runs #1642

Merged
merged 2 commits into from
Mar 11, 2024
Merged

Fix interpolate! for MPI runs #1642

merged 2 commits into from
Mar 11, 2024

Conversation

Sbozzolo
Copy link
Member

interpolate! did not allow dest to be nothing, as we would have in MPI runs (we only allocate dest on root).

This fix is blocking me from using it in ClimaAtmos, so I am also making a patch release.

For MPI runs, dest does not to be an array. Quite the opposite:
all the processes except root will not have an array as destination,
but they will still call `interpolate!`
Comment on lines +783 to +785
if !isnothing(dest)
# !isnothing(dest) means that this is the root process, in this case, the size have
# to match
Copy link
Member

Choose a reason for hiding this comment

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

What happens below if this is not the root process?

Copy link
Member Author

Choose a reason for hiding this comment

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

Each MPI process has to run the code below. Each MPI process performs interpolation of its local points, then there is a MPI reduce! call to move everything to root and save it to dest.

@Sbozzolo Sbozzolo enabled auto-merge March 11, 2024 17:54
@Sbozzolo
Copy link
Member Author

ClimaCore 0.13.1 running in Atmos with interpoalte!:

https://buildkite.com/clima/climaatmos-ci/builds/17355

Everything passes.

@Sbozzolo Sbozzolo merged commit a594d69 into main Mar 11, 2024
15 checks passed
@Sbozzolo Sbozzolo deleted the gb/faster_interp branch March 11, 2024 18:21
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