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

Mfvsinks #56

Closed dhubber closed 8 years ago

dhubber commented 8 years ago

Multiple changes to meshless scheme to

giovanni-rosotti commented 8 years ago

I can't accept it; there are conflicts to resolve...

dhubber commented 8 years ago

Okay, I just resolved the conflict (I think) by merging with the master branch so should hopefully be fine now.

giovanni-rosotti commented 8 years ago

Tried to compile and got the following error: src/MeshlessFV/MeshlessFVSimulation.cpp:1006:12: error: use of undeclared identifier 'level_max_sph' assert(level_max_sph >= 0);

Is this expected? (notice that I had DEBUG_LEVEL=1, which is probably why Travis didn't pick it up)

dhubber commented 8 years ago

ok, fixed that bug. Was due to variables named SPH instead of mfv/hydro. I also had some other OpenMP-related bugs with DEBUG_LEVEL=2 which I fixed, but only on my laptop compilers. Hopefully will now compile.

giovanni-rosotti commented 8 years ago

Perfect, can confirm that it now compiles. I've run some simple tests with the meshless and they seem fine. However some parameter files have values for "slope_limiter" that GANDALF now doesn't like anymore. What is a good default to provide? Does it depend on the problem? Or should we just remove it alltogether from the parameter files we provide, and just stick with the GANDALF default?

dhubber commented 8 years ago

Ok, good finally. A good default is 'gizmo' or 'springel2009' (gizmo perhaps is better).

On Monday, 21 March 2016, giovanni-rosotti notifications@github.com wrote:

Perfect, can confirm that it now compiles. I've run some simple tests with the meshless and they seem fine. However some parameter files have values for "slope_limiter" that GANDALF now doesn't like anymore. What is a good default to provide? Does it depend on the problem? Or should we just remove it alltogether from the parameter files we provide, and just stick with the GANDALF default?

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/gandalfcode/gandalf/pull/56#issuecomment-199194637

giovanni-rosotti commented 8 years ago

All right, then I'll go through the parameter files and change it to 'gizmo'

giovanni-rosotti commented 8 years ago

Wait, I've spoken too early. The boss bodenheimer test actually crashes immediately after IC generation, with some problem related to Ewald...

dhubber commented 8 years ago

Oh Ewald really? That's a bit strange because the Ewald is not even used by the Boss Bodenheimer test. Unless it's because the object is not allocated in the Meshless ProcessParameters?

rbooth200 commented 8 years ago

Silly question: Is this related to the changes to the Ewald class that I made when doing the boundary conditions stuff?

On 21/03/16 10:13, dhubber wrote:

Oh Ewald really? That's a bit strange because the Ewald is not even used by the Boss Bodenheimer test. Unless it's because the object is not allocated in the Meshless ProcessParameters?

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/gandalfcode/gandalf/pull/56#issuecomment-199210839

giovanni-rosotti commented 8 years ago

Possibly, but most likely some change was not propagated correctly to the meshless. I pushed a change suggested by David and it now starts. But after a while it crashes after a while with an assertion failed: a particle has a negative u! (MeshlessFV.cpp, line 457)

dhubber commented 8 years ago

ok, is this still the Boss-Bodenheimer? It should not really go negative because it's temperature is set as a barotropic equation of state. Also, I run the boss-bodenheimer before I committed and this didn't happen. I will try it again now and see what happens.

giovanni-rosotti commented 8 years ago

Yes, that's still the boss-bodenheimer. Well, I don't know, the only change I did is that I set sim=meshlessfv. Did you have DEBUG_LEVEL=1? If it was 0, there are no assertions.

dhubber commented 8 years ago

No, I didn't. I had DEBUG_LEVEL = 0. The way the thermodynamics works is it allows the Riemann solver to set the energy flux and integrate the energy, but at the end of the step, the temperature is re-set based on the Barotropic value (this is how things are done in grid codes as far as I know). It still shouldn't become negative for this test though so it's curious. I'll have a look now.

dhubber commented 8 years ago

Ok, I've spent quite a few hours on this now and I think this has exposed some genuine problem rather than just some trivial compiling issue. It seems the simple approach to using non-adiabatic EOSs doesn't work so well. At first, I thought the negative energies was due to the assert being called before the energies were properly updated by the EOS (since they are overwritten), but once I figured some things out, I started getting negative masses as well which is clearly not right. I've spent most of the day trying to find if there was some underlying bug (and I did find and fix a few small ones) but I've not found anything significant really. So, I'd probably suggest for now to stop the code (via an exception) if a non-adiabatic EOS is used with the MFV and fix this later (when I am Cambridge perhaps)? We can then can finally merge this branch, which has a lot of other useful-working stuff that ought to be in the master branch really.