Closed rbooth200 closed 7 years ago
This is fine. Just a quick question before merging: had we really not implemented the change already? Could it be that we have the similar commit in another branch we haven't merged yet?
I think there might be another fix in papertests, you could use that one instead if you prefer.
Do you mean 008cc7f? Do I understand correctly that the extra 0.5 you added here is actually commented out in that commit, and is added directly only to the dudt of artificial viscosity? Because if yes, when we will merge everything we will get it two times...
Yep I've just checked and that appears to be correct. Lets just forget this and try to merge papertests asap? I've checked with the evrard test in paper tests and the energy conservation seems fine,
ok agreed, let's do as you suggest. but i'll leave the pull request open until i've fixed papertests to remind me that i should action it quickly
I've tidied up the fix so it's like the one in paper tests and checked that the energy conservation is good in both cases. I think this can be merged now that we are going to use papertests2
This is the missing energy fix