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 sink potential check #234

Merged

Conversation

conradtchan
Copy link
Collaborator

Type of PR:
Bug fix

Description:
Required for #217, discovered when mpirun -n 4 was added

In ptmass.F90, during sink creation, whether the particle is at the potential minimum is checked:

       ! CHECK 7: particle i is at minimum in potential
       if (poteni > potenj_min) then
          ifail = inosink_poten
          ifail_array(inosink_poten) = 1
       endif

This check fails because an identical particle distribution gets created on every task, resulting in close comparisons when determining which particle to create a sink at.

This is fixed by only creating the particles on the master task, and allowing derivs to rebalance them.

itest needs to be calculated after calling derivs or else it will be incorrect. The xcofm calculation has been removed because the result is not used anywhere.

Testing:

make MPI=yes SETUP=testgrav phantomtest
mpirun -n 4 bin/phantomtest ptmass

Did you run the bots? yes, pre-commit

and rely on rebalance to distribute to other tasks
amend previous commit
remove cofm calculation which does not appear to do anything
@conradtchan
Copy link
Collaborator Author

Note that the automated test suite does not cover the bug that this PR fixes because #217 has not been merged yet. It needs to be tested by running the commands given in the description.

@conradtchan conradtchan marked this pull request as ready for review February 16, 2022 23:58
@conradtchan conradtchan merged commit d40b5a6 into danieljprice:master Feb 17, 2022
@conradtchan conradtchan deleted the fix-sink-potential-checkl branch February 17, 2022 00:02
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.

1 participant