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

Papertests2 #72

Closed dhubber closed 7 years ago

dhubber commented 7 years ago

Cleaner branch containing new files, sub-directory structures, parameters files and python scripts for gandalf paper tests and figures. Created from previous papertests branch via cherry-picking relevant commits.

rbooth200 commented 7 years ago

Looks good. I believe we are still missing the factor of 2 fix in the SPH energy equation. I preferred the solution from the papertests branch to the one I uploaded. Should we add that commit here and close the other pull request?

dhubber commented 7 years ago

Ahhh ok, I deliberately left that one out because I thought you'd done it so no point in duplicating and risking a conflict! Looking at it in more detail. there were also other parts nothing to do with the energy equation part such as for dust tests (it was quite a big commit) so maybe that one should be committed also. But feel free to commit the one you think is best, especially if it comes to this other stuff.

rbooth200 commented 7 years ago

I'll have a look at. Hold on a sec...

rbooth200 commented 7 years ago

I don't see anything dust related in that commit. There were some cosmetic changes, including some IC stuff related to dust and a load of Evrard test files added as well. I think you have added a bunch of these separately, but some are still missing (e.g. snap_008).

Since I think you have added some of the stuff from this commit separately perhaps we should not include it directly. Instead I could modify the pull request for the factor of 2 and you could fix the missing files here? Is that the best solution?

dhubber commented 7 years ago

Yes exactly, related to dust tests like I said :-). I realised also that I added them separately but I wasn't sure about whether we want snap_008 committed or not so did not add that one (but guess so if we want to be able to generate the figures, although we should rename the file to something sensible). I don't actually seem to have snap_008 here though so maybe best if you could do it. What other missing files btw?

rbooth200 commented 7 years ago

I've added the evrard solution file which was my langrangian hydro code solution. I will now sort out the energy fix on the other branch

dhubber commented 7 years ago

ok, I think everything on this branch is fine now (just tested the recent changes and all okay) so if everyone's happy, I guess we can merge this one also.

rbooth200 commented 7 years ago

I'm happy with the branch now and think we could merge as long Giovanni is happy with it.

giovanni-rosotti commented 7 years ago

Yes I went through the commits and I'm fine. I think the only commit left out is ab5049c, but that is correct because it has to do with tests and not with papertests. So I'll now accept the pull request and delete the branch.