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

Deleted MPI barriers in computeQdot #246

Merged
merged 4 commits into from
Dec 20, 2024
Merged

Deleted MPI barriers in computeQdot #246

merged 4 commits into from
Dec 20, 2024

Conversation

loganoz
Copy link
Owner

@loganoz loganoz commented Dec 11, 2024

Deleted MPI barriers in computeQdot and computeQdotHO to enhance MPI performance. It might break some implementations without MPI test in CI.

@loganoz loganoz requested a review from Jerryntk December 11, 2024 15:37
@loganoz
Copy link
Owner Author

loganoz commented Dec 11, 2024

@Jerryntk, take a look, but it would seem it's not creating any problem. Check also that I commented the correct line.

Decrease number of mpi partitions so it is under the number of mesh elements in the case
Decrease number of MPI threads for UniformFlow and Convergence. These meshes are very small.
@loganoz
Copy link
Owner Author

loganoz commented Dec 19, 2024

@AbbBallout , I am getting errors in the new multiphase Vreman test case. May be you were too optimistic with the threshold for detecting an error? Please double check.

@AbbBallout
Copy link
Collaborator

Only the gfortran release didn't pass.
The test passed after running again. Could be a race condition or something.

@loganoz loganoz merged commit 050e93b into master Dec 20, 2024
20 checks passed
@loganoz loganoz deleted the MPI-barrier-test branch December 20, 2024 08:50
@loganoz
Copy link
Owner Author

loganoz commented Dec 20, 2024

@AbbBallout, yeah, I think you are probably right. Can you take a look? Not urgent, so just right in your todo list and take a look when you have some extra time. Thanks!

@AbbBallout
Copy link
Collaborator

@loganoz Yeah sure will

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