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

Periodic gravity #62

Open rbooth200 opened 8 years ago

rbooth200 commented 8 years ago

There is still a small bug in the periodic gravity routine. This is related to the fact that the nearest periodic neighbour to a cell centre is not necessarily the same as the one closest to a particle within the cell.

This means that we need to make sure that we have the periodic corrected distance on a particle by particle basis.

rbooth200 commented 8 years ago

The Ewald correction is also not included when stars are present. It also seems to neglect the quadropole term and might be in error with the fast monopole too.

giovanni-rosotti commented 7 years ago

See #73, I think at least now the Ewald correction is present also when there are stars. Is the other staff still a problem or has it been corrected in the meanwhile?

dhubber commented 7 years ago

This first issue might be what caused the problems I had with the Jeans test which forced me to effectively go to brute-force. Is there a simple solution to this that we've simply not done?

And yes, the Ewald correction is now made also with the stars. I'll need to double check it's handled by all cases though before considering that one solved.

rbooth200 commented 7 years ago

I think this bug was referring to the edge case where a particle / cell sits nearly half a box away from the cell that we are computing the force for. In this case we can think that dr[k] should be about +0.5 boxsize[k], when really it should be about -0.5 boxsize[k]. To correct for this we need to call NearestPeriodicVector in the gravity tree routines for each particle pair when periodic boundaries are used

giovanni-rosotti commented 7 years ago

The issue with the cells too big has been fixed in #115 . Keeping the issue open until we have fixed the Nbody in #114

rbooth200 commented 7 years ago

What is the status of the ewald correction with the stars now? I think #114 is not going to be merged now since we have most of the commits.

If possible I'd like to see the ewald correction for the stars being optional in non-selfgravitating simulations. This is useful for cases where periodic boundaries are being used to simulate an infinite medium such as the ISM, but we are interested in a single set of star clusters. The binary accretion test is one case where this would be useful.

dhubber commented 7 years ago

What is the status of the ewald correction with the stars now? I think #114 is not going to be merged now since we have most of the commits.

I think it's working now. I tried the Jeans test with only stars (so in effect the limit of the Jeans length becoming zero) and it seemed absolutely fine. #114 has already been closed since as you say it's effectively already been merged into master elsewhere.

If possible I'd like to see the ewald correction for the stars being optional in non-selfgravitating simulations. This is useful for cases where periodic boundaries are being used to simulate an infinite medium such as the ISM, but we are interested in a single set of star clusters. The binary accretion test is one case where this would be useful.

Yes, I'll need to check exactly what happens with and without self-gravity switched on, but the case you describe is what we would like. I'll have a look at it soon.