- changed status to open
- removed comment
WeylScal4 schedule order is incorrect
WeylScal4 contains a calculation for computing the I and J invariants from the Psis. This calculation is not scheduled explicitly after the Psis. Therefore, Cactus will probably schedule the routines alphabetically, which should be wrong. This should be corrected either with an After or Schedule element in the calculation.
Keyword: WeylScal4
Keyword: testsuite
Keyword: backport
Comments (12)
-
-
reporter - changed status to open
- removed comment
Thanks. Looks good! Please apply.
-
reporter - removed comment
#1182 is an enhancement ticket for adding the test cases for the invariants.
-
- changed status to resolved
- removed comment
Applied.
-
reporter - changed status to open
- removed comment
I think this is a candidate for backporting to the release. My only concern is that computing the invariants pointwise rather than using boundary conditions will change numerical results, and is an enhancement not a fix. Additionally, the change in name of the functions could cause other thorns which rely on these names for scheduling purposes to break. I would backport the fix to the scheduling order only. What do others think?
-
repo owner - removed comment
Can we separate out the bugfix from the ehancement? Ideally we would also not re-release until the tests pass on at least some of the machines (some that are convenient or important I would guess). We don't want to accidentally break the release branch.
-
reporter - removed comment
I have set up automated tests for the release branch (https://build.barrywardell.net/job/EinsteinToolkitReleased/). These are only run on a single machine (a ubuntu VM), not a production machine. The tests will only be run AFTER we commit to the release branch. And at that point, the code will be downloaded by anyone checking out the released version, as we tell people to check out by branch not by tag. I have moved this discussion to the mailing list, as it affects a wider audience.
-
- changed milestone to ET_2013_05
- removed comment
-
reporter - removed comment
For the upcoming release, we should fix the schedule order only. The pointwise computation and the renaming both break backward compatibility.
-
- removed comment
Isn't this patch already applied, and thus, also the point-wise calculation? It seems that this bug was only re-opened to get the schedule-change into the 2012_11 release, which by now is kind of pointless. Can this bug be closed again?
-
- changed status to open
- removed comment
-
- changed status to resolved
- removed comment
This has been applied already some time ago.
- Log in to comment
In addition to correcting the schedule, the enclosed patch also adds a "WeylScal4_" prefix to all schedule items, and evaluates the invariants pointwise instead of applying boundary conditions.
We should also modify one of the test cases to calculate the invariants as well.