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

Visc #146

Closed rbooth200 closed 7 years ago

rbooth200 commented 7 years ago

This branch, based off of the mfv_dust2 pull request implements two changes to the viscosity:

1) An addition of physical viscosity in the meshless, to add viscosity where desired 2) An implementation of the Cullen & Dehnen switch in the SPH, to remove viscosity where not desired.

When the mfv_dust2 branch is ready to go then this one should be too.

rbooth200 commented 7 years ago

I changed the base to mfv_dust2, so you can see just the new changes in this branch...

dhubber commented 7 years ago

Oh, this is very interesting and useful (particularly the viscosity for the meshless). Is this being rushed through so we can also add it in the paper? If so, we'll have to expand some of the tests a little, such as extra shocktube tests for the CD viscosity.

rbooth200 commented 7 years ago

Yes, I agree that this needs testing. It should be easy enough to add a nose test for the CD test tube. I'll do that soon...

But I think giovanni plans to explicitly test them for the paper using the spreading ring test. I guess we could also add a nose test for the spreading ring, but Giovanni would need to add his 'analytical solution' for comparion.

rbooth200 commented 7 years ago

The problem with this line has been that floating point errors seems to cause the assert to fail when it should pass with equality. That's why we've been replacing it with the 0.99, after which the iteration over h and neighbours takes care of it.

On Fri, 26 May 2017, 09:28 dhubber, notifications@github.com wrote:

@dhubber commented on this pull request.

In src/GradhSph/GradhSph.cpp https://github.com/gandalfcode/gandalf/pull/146#discussion_r118656384:

@@ -207,7 +207,7 @@ int GradhSph<ndim, kernelclass>::ComputeH

 // Density must at least equal its self-contribution
 // (failure could indicate neighbour list problem)
  • assert(parti.rho >= parti.mparti.hfactorkern.w0_s2(0.0));
  • assert(parti.rho >= 0.99parti.mparti.hfactor*kern.w0_s2(0.0));

I've had some bug-reports regarding this line when the computed density is exactly equal to the self-contribution. It suggests some other problem (think it's to do with the sinks which I've solved in another branch) but also it could be the guess is simply too small so there's only the self-contribution, so I think making this slightly smaller than 1 is fine to stop this assert killing the code when there's not a problem.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/gandalfcode/gandalf/pull/146#pullrequestreview-40469584, or mute the thread https://github.com/notifications/unsubscribe-auth/AOqItFPNCofKK9EYZRB3gurk6xjQOAcuks5r9o1IgaJpZM4Nfs7o .

dhubber commented 7 years ago

Okay, I've had a look through the branch and also tried it out on a couple of simulations. However, I didn't really see much difference at all between the MM97 and CD2010 switches. I only really tried a shocktube test (where perhaps I should not expect a big difference) and the shear flow test (where I would perhaps expect a bigger difference). Or maybe these tests are not challenging enough to show any differences? I'm guessing you're preparing some more sophisticated tests for both the CD2010 and the physical viscosity cases? Otherwise there were no problems and the coding seemed fine so I'm happy with it.

rbooth200 commented 7 years ago

I agree that (hopefully) there should be little difference in the shock tube tests. Normally I'd use the gresho when exploring viscosity. I'll have a look later.