@pgrete , there are actually different errors (or at least additional errors) in this last set of tests. Would you mind looking at that?
All the extra failures are due to the gold standard not being updated after several recent PRs that change the gold standard were merged. I've set up a simpler way for me to update the gold when needed, so now it should be easier to update these faster. Re-testing on the current PRs are running now.
It occurs to me that it would be helpful to cite, in the source code, the paper where this is all spelled out. I've added a few places where it's not particularly obvious what you're doing.
okay, we're back to all the original errors (really just differences), so i think we're back on track to get reviews from the above folks. ping @dcollins4096 @pwang234
The latest update incorporate (hopefully) all previous comments.
I had to extend the RNG (by save and restore functions) in order to being able to resume stochastic forcing simulations in a deterministic way.
These are actually the same error as before - it's just differences from before, and is just fine. @dcollins4096 , @pwang234 -- can you both take a quick look at this?
It looks great. The only thing I think might be missing is proper support for MHD/MHDCT and <3d, since both MHD methods always require Vy and Vz. (This is a rare case, of course, but useful if one wants to do, say, ambipolar diffusion) I think I caught all the bits that I know will crash, but if you could double check if the anything else will choke if GridRank <3 and Vy or Vz exist that would be grand.
I added MHDCT support and tested serial/MPI runs incl. restarts.
In addition to changing the problem initialization file, I had to make further changes as Grid_*SourceTerms.C is only called by the MUSCL solvers.
I considered to the existing Grid_AddRandomForcing to be the most appropriate place and changed it so that both types for forcing (Random and Stochastic) are supported along the same lines of code.
Please have a second look, whether I missed something there or in case you think that these should be kept separate.
So for PPM/MHD_Li runs, the old Grid_AddRandomForcing is used to drive, while for the MUSCL, the Grid_*SourceTerms.C is used, is that correct?
That's correct. However, if there's a nicer way to handle this, I'm open to suggestions.
I think that's as nice as it gets. Thanks for the clarification.
@dcollins4096 , is there any reason that we should not merge these changes?
I don't see any reason to not merge this, but there's still an outstanding failure from Jenkins. The report looks like noise. I'll merge this tomorrow if nobody says I shouldn't.
@bwoshea, can you make sure that the answers get regenerated before we merge?
@dcollins4096 , please hold off on hitting merge. I am going to regenerate the gold standard and test this PR vs. the regenerated standard. I expect it to pass - the problem with Jenkins is expected - but it's better to be safe than sorry. I will do it over the weekend if I can, but more likely Monday morning.
If someone tells me that it is okay to regenerate the gold and bump the version on jenkins, all the failures will dissappear. I sent an email/left a comment on another PR a while back (to the list or @bwoshea maybe?) asking for someone to test a PR that got merged while Jenkins was down and has floating point differences that lead to all the current errors from what I can tell.
Hi @samskillman , if you want to regenerate the gold standard and bump the version on jenkins, that would be great. Thank you!
OK, following up on this PR: in the new gold standard (http://srs.slac.stanford.edu/hudson/job/enzo-dev/239/) there are 33 test failures. These are the same test failures as before, and occur only in the DrivenTurbulence3D test problem. I concur with @pgrete that this is due to differences in the treatment of energy. The differences are at an acceptable level, and I am going to merge this PR momentarily.