For what it's worth, I don't see the failures on the PhotonShadowing test you pointed out. I ran the answer tests using a locally generated gold standard from the current tip of enzo-dev, including strict tolerances and bitwise tests. My test results are pasted here: http://paste.yt-project.org/show/3362/
I do see failures for the Orszag Tang test - but seeing how you updated that test below, I suspect that is the expected behavior and that the gold standard will have to be regenerated once this is accepted.
So I finally ran the full push suite for MHDCT using a locally generated gold standard, strict relative tolerances, and tests for bitwise identicality.
In addition to the failure you noted for PhotonTestAMR (which I also see on the enzo-3.0 fork) I also see failures for NohProblem2DAMR as well as the two new Orszag Tang tests you've added. Of course since the Orszag Tang problems are not included in the current answer testing suite, I'm not surprised about these failures. The test results are pasted here:
The differences in NohProblem2DAMR are small but a bit worrisome. For comparison I am able to get bitwise identical results compared to my local gold standard using my enzo-3.0 PR. Please let me know if you need any more information to reproduce this behavior on your end.
Dave -- I just wanted to add two things: (i) this is fabulous work! I've been spending some time looking at this while working on the method paper and I'm really excited about getting it merged in. (ii) Matt and I have been working on putting the new Field object into this, to cut down on some of the new code, particularly in Grid_CommunicationSendRegion and Grid_CommunicationReceiveRegion. Hopefully we'll be ready to bring that in the next week or two (but if it's more than that, then this shouldn't hold up acceptance). Thanks for the great work!
This sounds like a good plan regarding the Field object project. Could we possible set a firm deadline for getting this code into Enzo and tested, so the MHDCT pull request doesn't hang in limbo? It's April 17th today... how about May 1st?
Greg and I have discussed it and as a result of circumstances beyond our control, we have been unable to make this deadline. We're going to hold off on making changes or further requests of this pull request. However, as a compromise, we will be pushing on field objects in the "enzo-dev" branch (which is distinct from the enzo-dev repository), and request that the MHDCT be held back from enzo-dev until we have finished that work, so that the MHDCT changes can be integrate with those. We will handle that pull request and modifications to the code to ensure that happens.
I tried to reproduce that error when Nathan ran into it, and wan't able to make it fail. Nathan concluded that it was an accumulation of roundoff error. Do you think that's the case for your run, or is this a real problem?
the PR is now updated after merging dave's fork with the enzo-dev tip
enzo-dev with all of the MHD-CT changes passes the push suite, and produces bitwise identical results with the exception of Noh2DAMR, which is off in the ~8th decimal place for some people but not others. This is a bit mysterious, but has been deemed "not a show-stopper," and I don't see it on my own machine so I can't debug it.
We have a plan to move forward with the baryon field objects: the field objects are being implemented in enzo-3.0, and when the MHD is pulled from enzo-dev (really, the enzo-2.x development repository) into the enzo-3 development repository, it will be updated to include field objects.
So, unless somebody has a substantial objection, I will accept this PR later this afternoon. Speak now or forever hold your peace! :-)