search

gandalfcode / gandalf

GANDALF (Graphical Astrophysics code for N-body Dynamics And Lagrangian Fluids)
GNU General Public License v2.0
44 stars 12 forks source link

MPI sinks/N-body #25

Open dhubber opened 10 years ago

dhubber commented 10 years ago

Ensure N-body and sinks are correctly computed and communicated between MPI nodes

giovanni-rosotti commented 8 years ago

It should be fine - but needs to be properly tested

giovanni-rosotti commented 7 years ago

There was an issue with the gravity of the stars onto SPH particles being counted multiple times which I've fixed in #115 (done in that branch because it modifies GradhSphTree which has been heavily modified in that branch). Now energy is conserved in the hybrid plummer test (it wasn't before) which is promising. An analytical solution would help though; do we have one?

I'll now give a look at sinks. I think we need a test for that as well. There's a bondi accretion test, but it uses half a million particles (!) so not exactly what we want...

rbooth200 commented 7 years ago

To test the sinks can't we just do something stupid like setup one simulation where we put a load of particles inside the sink radius and check whether they are accreted? We could do a second one where the particles are moving at high velocity and are unbound, so shouldn't be accreted.

giovanni-rosotti commented 7 years ago

Yes I'll implement a stupid test like this, and also check how much I can reduce the resolution of the bondi accretion test before it goes crazy. There were problems with the sinks that I should now have fixed - hopefully the tests will tell me if there is any other problem.

Regarding the Nbody dynamics, I'll probably also add some check on the hybrid plummer (i.e. that the lagrangian radii behave more or less like figure 4c of Hubber et al 2014).

Finally, two problems I have found in the sinks: 1) sink accretion will not work correctly if there are periodic ghosts, because the changes to the periodic ghosts are not propagated back to the real particles. Should I fix it? Are periodic simulations with sinks useful? 2) in SPH, we do not currently update the ghosts before doing accretion, and we should. This is connected to #96, which was about the meshless, but actualy SPH has the same problem. The only moment in which the ghosts are up to date in SPH is soon before the h calculation. Should we do sink accretion at that point? It will use the old h, but maybe it's not the end of the world...

rbooth200 commented 7 years ago

Sounds good. My thoughts about your questions are as follows:

1) Yes. Some people might want to do periodic turbulent star formation simulations

2) Urgh. Ideally I'd like to have a solution that is self-contained in Sinks.cpp, that all code can use robustly. Maybe its too much work, but it goes together with what we said about the N-body integration...

giovanni-rosotti commented 7 years ago
  1. fine, I was hoping to avoid it but it seems I will have to do it :). it's just an easy fix anyway
  2. ideally, i would like as well (and indeed i've removed some initialisation code from the main loop and moved it into sinks.cpp). but updating the ghosts comes with a high prize, and we should really avoid doing it as much as possible. What we are doing at the moment is wrong as the ghost might be in a different position than the real particle... Do you see any problem in doing sink accretion before the h calculation?
rbooth200 commented 7 years ago

I don't see an immediate issue with doing the sinks first. It makes sense to do them at the start or end of the time-step, but I don't mind which.

In the meshless I implemented a 'need density' flag to avoid updating the density when it's not needed. Would something like this be useful for the ghosts?

giovanni-rosotti commented 7 years ago

I was thinking about that. The problem is a) everything that invalidates the ghosts needs to update the flag b) what do you do if the ghosts are not up to date? for correctness you should update, but how does the user know that things are much more expensive than they could be with a different rearrangement? Or do you just crash, saying "rearrange the main loop"? I don't want to do an expensive operation silently...

rbooth200 commented 7 years ago

I'm happy to leave it for now if there is no obvious way to do it.

giovanni-rosotti commented 7 years ago

See also #122

rbooth200 commented 6 years ago

Re-visiting this, it's clear that in discs the sink radius can be much larger than the smoothing length of the particles so we can easily end up missing the particles that should be accreted when using the ghosts to handle the sink accretion. I think this means that we need to switch to computing the sink accretion on every processor, and doing a reduction on the total quantities. I think this can also be made to work for smoothed accretion.