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

Improve ics2 #177

Closed dhubber closed 6 years ago

dhubber commented 6 years ago

Following improvements/fixes already added :

A few possible changes to come before this should be merged

Any other suggested IC class improvements that could go with this pull request (but nothing too big obviously)?

rbooth200 commented 6 years ago

Add GetDensity and SetProperties function to some other IC classes to allow regularisation to be used (Giovanni, Richard : could the DiscIc class be changed to use the regularisation? It starts off with random sampling which could instead be done by the other functions in the IC class and then regularisation could 'clean-up' the noise)

Regularization could be added, but I would not make the disc IC use other functions in the IC class. The disc IC class needs to draw particles from special distributions, I see no point in changing something that works and is a special case. There is, however, an analytic form for the density so you could use that for the regularization, if you wish.

dhubber commented 6 years ago

Regularization could be added, but I would not make the disc IC use other functions in the IC class. The disc IC class needs to draw particles from special distributions, I see no point in changing something that works and is a special case. There is, however, an analytic form for the density so you could use that for the regularization, if you wish.

Sure, if it's set-up to work then no need to change the random sampling. But surely it would still work with the Monte-Carlo rejection sampling?? Technically it's slower because it has to repeat but it should give the same quality of ICs surely? But yes, I'm mainly asking purely to enable regularisation to decrease the start-up noise.

Btw, there's some strange issue that's causing all the soundwave test cases to fail which I don't understand at all (it works fine on my laptop and the test is extremely simple) so I'll open a new branch and add my changes one by one to figure out why it's failing!

rbooth200 commented 6 years ago

This might be useful: https://git-scm.com/docs/git-bisect

rbooth200 commented 6 years ago

Sure, if it's set-up to work then no need to change the random sampling. But surely it would still work with the Monte-Carlo rejection sampling?? Technically it's slower because it has to repeat but it should give the same quality of ICs surely?

The current code is doing rejection sampling.

dhubber commented 6 years ago

This might be useful: https://git-scm.com/docs/git-bisect

Yes, I knew about this for the command line (and should use it more often), but can it be used in some way on git-hub with Travis that doesn't involve pushing from my laptop all the time? It actually works perfectly fine on my laptop; it's only breaking when it goes through the test suite on Travis :-/

rbooth200 commented 6 years ago

Are you valgrind clean?

dhubber commented 6 years ago

Hmmm, I don't know. I don't often use Valgrind (since it's so bloody slow) but might be a good call to use it in this case.

dhubber commented 6 years ago

Closing and moving effort to different branch.