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 ghost particle exchange optional #1253

Merged
merged 1 commit into from Oct 19, 2016

Conversation

gassmoeller
Copy link
Member

I have observed that in some models with many particles the exchange of ghost particles is quite expensive. It might be possible that there is a bottleneck in the code that I do not quite understand, but so far I have not observed this behaviour in applications (only in tests). Therefore I would like to disable the exchange by default for now, until we figure out what is going on. @naliboff, @egpuckett, @hlokavarapu: This will likely require you do add the parameter 'Update ghost particles = true' to your models with active particles, sorry for that inconvenience. Did you ever had a model in which the exchange of ghost particles took longer than all of the rest of the particle algorithm combined?

@bangerth
Copy link
Contributor

OK to merge once tester is happy.

@hlokavarapu
Copy link
Contributor

Thanks for letting us know!

Here is a plot regarding the timing of the exchange of ghost particles from data computed by myself and @egpuckett .

solkz_particle_statistics

Essentially, with the grid fixed, I have increased the number of particles by a factor of four from run to run keeping the number of MPI processes constant at 2. As expected, the wall clock time of the exchange of ghost particles increases by a factor of four from run to run. However, in the last case, where the number of particles is larger than 1e7, the computation was ran with 4 MPI processes as opposed to 2. Yet, the increase in wall clock time compared to the previous run is still a factor of 4. Perhaps, the implementation is not scaling as expected? Have you ( @gassmoeller ) considered running strong scaling tests to better understand the components of the current implementation? In fact, I don;t mind running these tests if you believe that it can provide some insight.

@gassmoeller
Copy link
Member Author

Yes this looks pretty similar to what I am observing. I did some strong scaling tests for all the other parts of the particle algorithm, but have not done so for the ghost exchange yet (and currently do not have the time to do so and then optimize this part of the code), so feel free to go ahead and do some tests. There are three parts of the algorithm that might take longer than I expected:

    1. the identification of which particles to send
    1. the call to send_recv_particles, which ships them around between processes
    1. the following insertion into the ghost_particles map on the new process

I guess it would be most useful to create timing section for the three subparts (put them in their own scope { ... }) and then do the scaling tests to see which one causes the trouble (maybe do a single run first, maybe it is not a scaling issue at all, but just one part is very slow).

@gassmoeller gassmoeller merged commit 5914c7b into geodynamics:master Oct 19, 2016
@gassmoeller gassmoeller deleted the particle_optimizations branch October 19, 2016 18:06
@gassmoeller
Copy link
Member Author

@hlokavarapu: After all I was forced to do something about the exchange ghosts part (I want to run a large model and this function would eat up all of our allocation). Hope you have not spend time on it so far. It turns out the problem was within the serialization/deserialization of CellID data in deal.II, which is called inside of send_recv_particles. I openend a pull request there (dealii/dealii#3289). When this is merged the exchange_ghosts function should be around 5 times faster.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants