Final paper tests - Githubissues

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

Final paper tests #135

Closed dhubber closed 7 years ago

dhubber commented 7 years ago

This pull request is pretty bulky and is a combination of various other branches and important fixes. In summary, the most important fixes are :

The main IC refactoring, moving individual ICs into their own classes.
Regularised ICs, both to form a glass and generate an arbitrary density field from a function
Various fixes to Ewald class (including the initialisation bug)
Changes to the EOS/FV classes, in particular for isothermal EOSs in the MFV scheme
Additional test files for the paper (including python scripts for figures)

rbooth200 commented 7 years ago

I'm happy enough with this...

giovanni-rosotti commented 7 years ago

I think this is so big that I cannot really inspect all of it; I gave a look around and I don't see any problem, but I can't claim I know all the changes that have been made. Clearly something like this should not happen again, but I know this was a special case. I guess the question is: why should we NOT accept it (i.e., are there known issues with this pull request, and if yes, are they showstoppers)?

dhubber commented 7 years ago

Yes, I agree. It's definitely too large to inspect everything. As far as I know, it certainly does not introduce any major issues (and obviously fixes quite a few). It passes the Travis tests already. The only other thing I can suggest is you can try some non-Travis tests of something that's been a problem in a past for some extra verification (and peace of mind)?

rbooth200 commented 7 years ago

I'm happy to accept it this time given that it 'appears' to fix some bugs. I wouldn't say that it obviously fixes them since it's so large that we don't know what made the difference.

Lets try to keep one thing per pull request from now on...