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

Added safety check for integral gradients #138

Closed rbooth200 closed 7 years ago

rbooth200 commented 7 years ago

I noticed some crashes in the Boss-bodenheimer due to poor gradient estimation. Hopkins points out that this can occur in his paper, and proposes switching back to standard SPH gradients.

This branch implements a similar fix, using the 'constant exact' SPH gradients. The constant exact gradients are the best choice for fall back for two reasons: 1) They are the most accurate estimate that does not require a matrix inversion, and 2) they allow for a definition of the face areas (derivative of volumes) that is consistent with the normal meshless flux calculation.

dhubber commented 7 years ago

Definitely sounds like a reasonable fix to make, especially if there's a simple known solution we can use. Presumably (and hopefully) the crashes go away now with this fix??

rbooth200 commented 7 years ago

The crashes do go away, yes.

giovanni-rosotti commented 7 years ago

Looks reasonable to me, and I'm happy for it to go into master as soon as the test is finished. I'll leave the responsibility of accepting it to David as he is more into the meshless than I am.

dhubber commented 7 years ago

Sure, I'll have a look at it later (assuming there are no more commits to come Richard?) and will accept if everything runs fine.

rbooth200 commented 7 years ago

It's ready to go.

dhubber commented 7 years ago

Ok, almost all simulations run fine. The only issue I came across is when I run the Sod test with mirror boundaries; there were some warnings being printed out about the bad gradients and the timestep became very short. However, a similar thing happened with mirror boundaries with SPH so I guess this is actually a mirror boundary issue (not used them in a while tbh so perhaps one exists). So unless there are any objections, I will make an issue for the mirror boundaries but also accept this branch?

rbooth200 commented 7 years ago

Let me check. I'll get back to you asap

rbooth200 commented 7 years ago

This was a bug that found its way into the master when we implemented the neighbour finder. I've fixed it here since it was easy enough and this branch is ready to go.

dhubber commented 7 years ago

Great, works fine now on my laptop. Will accept once it passes Travis!