Hello all,
Ian Hinder found that the testsuite teukolskyID for WeylScal4 was poisoned. It turns out I had forgotten to add MoL to the list of ActiveThorns in the testsuite.
Attached please find a patch which rectifies that. Ok to apply (both development and release branch)?
Expect some more patches while Tanja and I try and add some extra statements to WeylScal4 to make sure MoL is present (ie. something like inherit from MoL maybe) and adapt to a possible future change in how MoL_PseudoEvolution is scheduled.
Ian also pointed out some issues with how (outer hopefully) boundaries are handled which will have to be looked at eventually.
Yours, Roland
On Jun 22, 2010, at 16:58 , Roland Haas wrote:
Hello all,
Ian Hinder found that the testsuite teukolskyID for WeylScal4 was poisoned. It turns out I had forgotten to add MoL to the list of ActiveThorns in the testsuite.
Attached please find a patch which rectifies that. Ok to apply (both development and release branch)?
Expect some more patches while Tanja and I try and add some extra statements to WeylScal4 to make sure MoL is present (ie. something like inherit from MoL maybe) and adapt to a possible future change in how MoL_PseudoEvolution is scheduled.
Inheriting seems like a good way at the moment. Alternatively you can use (share) one of MoL's parameters, as a real evolution thorn would.
This patch is safe since it only modifies a test case; it should be applied to both branches.
-erik
On 22 Jun 2010, at 16:58, Roland Haas wrote:
Hello all,
Ian Hinder found that the testsuite teukolskyID for WeylScal4 was poisoned. It turns out I had forgotten to add MoL to the list of ActiveThorns in the testsuite.
Attached please find a patch which rectifies that. Ok to apply (both development and release branch)?
Expect some more patches while Tanja and I try and add some extra statements to WeylScal4 to make sure MoL is present (ie. something like inherit from MoL maybe) and adapt to a possible future change in how MoL_PseudoEvolution is scheduled.
Ian also pointed out some issues with how (outer hopefully) boundaries are handled which will have to be looked at eventually.
Since I am currently working on enhancing Kranc to automatically generate the code to correctly do the right thing with the boundaries, this should make the current "generate then patch" strategy obsolete. As of now, with my enhancements to Kranc, I can generate a version of WeylScal4 which passes the testsuite. For the released branch, I suggest we just leave it as is. For the trunk, I think we will want to autogenerate the code correctly, so I wouldn't spend time on fixing up the patch.
On 22 Jun 2010, at 17:50, Ian Hinder wrote:
On 22 Jun 2010, at 16:58, Roland Haas wrote:
Hello all,
Ian Hinder found that the testsuite teukolskyID for WeylScal4 was poisoned. It turns out I had forgotten to add MoL to the list of ActiveThorns in the testsuite.
Attached please find a patch which rectifies that. Ok to apply (both development and release branch)?
I would be OK with that, on the basis that it is only the test which is affected, and the test wasn't meaningful before. (We really need to have some sort of formal verification tests in addition to regression tests.)
Expect some more patches while Tanja and I try and add some extra statements to WeylScal4 to make sure MoL is present (ie. something like inherit from MoL maybe) and adapt to a possible future change in how MoL_PseudoEvolution is scheduled.
Ian also pointed out some issues with how (outer hopefully) boundaries are handled which will have to be looked at eventually.
Since I am currently working on enhancing Kranc to automatically generate the code to correctly do the right thing with the boundaries, this should make the current "generate then patch" strategy obsolete. As of now, with my enhancements to Kranc, I can generate a version of WeylScal4 which passes the testsuite. For the released branch, I suggest we just leave it as is. For the trunk, I think we will want to autogenerate the code correctly, so I wouldn't spend time on fixing up the patch.
The attached patch to WeylScal4 (just the m directory; I haven't included the autogenerated files in the patch), to be used with the corresponding patches I sent to the Kranc list, removes the post- processing that was needed before to get the boundary treatment correct. I also added "methodoflines" to the list of inherited implementations, since the thorn relies on MoL_PseudoEvolution being present. Probably Kranc should make all thorns inherit from MoL, which would avoid this problem in future.
With the patches to Kranc and WeylScal4, WeylScal4 passes its existing testsuite (you have to delete a couple of now-unused parameters from the testsuite parameter file). Depending on comments, I would propose committing these changes to the trunk. There's no need to modify the release branch.
On 23 Jun 2010, at 01:33, Ian Hinder wrote:
The attached patch to WeylScal4 (just the m directory; I haven't included the autogenerated files in the patch), to be used with the corresponding patches I sent to the Kranc list, removes the post- processing that was needed before to get the boundary treatment correct. I also added "methodoflines" to the list of inherited implementations, since the thorn relies on MoL_PseudoEvolution being present. Probably Kranc should make all thorns inherit from MoL, which would avoid this problem in future.
With the patches to Kranc and WeylScal4, WeylScal4 passes its existing testsuite (you have to delete a couple of now-unused parameters from the testsuite parameter file). Depending on comments, I would propose committing these changes to the trunk. There's no need to modify the release branch.
Unless there are objections in the next few hours, I will make this commit to the trunk.
On 6 Sep 2010, at 16:13, Frank Loeffler wrote:
On Mon, Sep 06, 2010 at 03:04:50PM +0200, Ian Hinder wrote:
Unless there are objections in the next few hours, I will make this commit to the trunk.
Please go ahead.
I also need to commit three additional patches (attached):
weylscal4runmath.patch: Correct the "runmath.sh" script used to generate WeylScal4 to find Kranc correctly in an Einstein Toolkit checkout now that Kranc has moved into the repos directory
weylscal4tests.patch: Remove the obsolete parameters WeylScal4::Psi4r_group_bound and WeylScal4::Psi4i_group_bound from WeylScal4 test suites
weylscal4autogen.patch: Update the automatically generated code in WeylScal4
Any parameter file that uses WeylScal4 needs to have the parameters WeylScal4::Psi4r_group_bound and WeylScal4::Psi4i_group_bound removed (these parameters are no longer present and keeping them in the parameter file will cause the run to abort on startup). They were present in the old hacked WeylScal4 version for technical reasons and didn't do anything useful anyway. The example parameter file given with CactusNumerical/InterpToArray needs to be updated; I will send a patch to the Cactus patches list.
Hello Ian,
weylscal4runmath.patch: Correct the "runmath.sh" script used to generate WeylScal4 to find Kranc correctly in an Einstein Toolkit checkout now that Kranc has moved into the repos directory
I'd rather have the 'repos' part as part of KRANCPATHS otherwise it will look for kranc in $HOME/repos/kranc and ./repos/kranc and not $HOME/kranc and ./kranc. Attached please see a patch proposal. The complicated '..' constructs are really just there to have a path that does not contain just '..' to do a poor mans test whether we are in a Cactus source tree.
Yours, Roland
On 9 Sep 2010, at 12:34, Roland Haas wrote:
Hello Ian,
weylscal4runmath.patch: Correct the "runmath.sh" script used to generate WeylScal4 to find Kranc correctly in an Einstein Toolkit checkout now that Kranc has moved into the repos directory
I'd rather have the 'repos' part as part of KRANCPATHS otherwise it will look for kranc in $HOME/repos/kranc and ./repos/kranc and not $HOME/kranc and ./kranc. Attached please see a patch proposal. The complicated '..' constructs are really just there to have a path that does not contain just '..' to do a poor mans test whether we are in a Cactus source tree.
OK - I just tested your patch and it works for me. Thanks!
Anyone have any objections to committing these patches?
users@lists.einsteintoolkit.org
-
Erik Schnetter -
Frank Loeffler -
Ian Hinder -
Roland Haas