- removed comment
Kranc-generated thorns should be regenerated before the release
Thorns generated by Kranc (McLachlan, WeylScal4 and EinsteinExact) should be generated by the version of Kranc which is being released.
Keyword:
Comments (20)
-
-
- removed comment
It would have been go to do this also for the latest release (ET_2013_05), but unfortunately we've already missed the release window. There is talk of a creating a point release, so it might be a good idea to regenerate all Kranc-based thorns before that happens so that the changes make it in.
-
- removed comment
If we do this it would be good to know what that would actually change for the user.
-
- removed comment
Re-generating McLachlan and all other auto-generated codes is a no-brainer. I would consider it a bug if they are not up to date. The sources are the Kranc scripts; everything else just depends on this. On the trunk, we should just re-generate the code, and see if something breaks.
I don't see a reason to change the released version. It has been tested, it works, so let's keep it that way.
-
- removed comment
That's was what I was trying to find out: of course the thorns should be re-generated. However, changing the released version should have a better reason than that alone, at the very least something a user would actually gain from, but most likely something like a bug fix, and better a serious one.
-
- removed comment
In the light of this for the future: what would be a good way to do this automatically? Is there a good way? Doing this automatically would require a machine with valid license, plus commit credentials on disk. In addition, it should probably run the testsuites and only commit if they pass (or something alike). Of course, we could just continue as we did (but maybe a bit more careful around release-time) - by hand.
-
reporter - removed comment
Replying to [comment:4 eschnett]:
I don't see a reason to change the released version. It has been tested, it works, so let's keep it that way.
The reason is that it can cause surprise to users who might want to make modifications to the code. They will regenerate the thorn and see some unrelated changes. If the changes are minor (formatting etc), I think they should be included in the point release. If not, then we should just chalk it up to experience and try to do better next time. We should also warn people that these thorns might change if regenerated.
Replying to [comment:6 knarf]:
In the light of this for the future: what would be a good way to do this automatically? Is there a good way? Doing this automatically would require a machine with valid license, plus commit credentials on disk. In addition, it should probably run the testsuites and only commit if they pass (or something alike). Of course, we could just continue as we did (but maybe a bit more careful around release-time) - by hand.
The process should be that whenever changes are made to Kranc, an ET developer (probably the one who made the changes) should verify that the changes to the generated code look reasonable, and commit them. In case we forget, we should have an automated system which reminds us. We have the build VM at UCD which has access to a Mathematica licence server, and it has been on my to-do list for a while to arrange such a process. It should be straightforward. My plan would be to have the Kranc-generated thorns regenerated on the build server after relevant commits in a separate "build job", and any differences would be classed as "test failures" for that job, and would be reported to the usual channels. It would then be up to a developer to verify the changes and regenerate the thorn. I don't think they should be committed automatically. This would require the build server (which lives behind a web application) to have commit access to our code repositories, and it might be tricky to handle the case of conflicts. It would also require, as you say, that the build server run the tests on the newly-generated code, and have even more intelligence about what to do then. I think it is much simpler and safer to require manual intervention in this case.
Next time, the release process as listed at https://docs.einsteintoolkit.org/et-docs/Release_Process should be followed. This includes
Ensure that any manually-generated files are consistent (i.e. regenerate McLachlan, WeylScal4 and EinsteinExact from their source with the current version of Kranc)
-
- removed comment
Replying to [comment:7 hinder]:
Replying to [comment:4 eschnett]:
I don't see a reason to change the released version. It has been tested, it works, so let's keep it that way.
The reason is that it can cause surprise to users who might want to make modifications to the code. They will regenerate the thorn and see some unrelated changes. If the changes are minor (formatting etc), I think they should be included in the point release. If not, then we should just chalk it up to experience and try to do better next time. We should also warn people that these thorns might change if regenerated.
I haven't checked McLachlan or WeylScal4, but in the case of EinsteinExact the only change it makes is to the schedule.ccl, adding (Everywhere) to the READS and WRITES lines.
-
reporter - removed comment
-
There are no changes to McLachlan.
-
In WeylScal4 and EinsteinExact, some comments are removed from schedule.ccl files, and the READS and WRITES statements are modified to have locations added (i.e. "WRITES: WeylScal4::Psi4r" becomes "WRITES: WeylScal4::Psi4r(Interior)"). There is also a small documentation modification to EinsteinExact that I don't understand. I think these changes are all minor enough to backport to the release branch. The benefit of having a clean correspondence between the version of Kranc and the code in my opinion outweighs the risk of breakage in this case.
-
The Kranc examples were regenerated before the release, but after the release branch was created. The changes here are more substantial, but correspond to the changes which have been in McLachlan and WeylScal4 for a while. Given that, and the fact that the Kranc examples are not production-critical, it would also be nice to have these backported.
I have committed all the regenerated code to the trunk.
-
- removed comment
Just a bit related: do you have experience using different Mathematica versions - do they all produce the same result in the end, or do especially the mathematical expressions change between these? If they do, which wouldn't surprise me, users cannot expect identical results anyway when regenerating for themselves.
-
reporter - removed comment
Good point. Yes, the expressions changed between 7 and 8. I don't know about between 8 and 9. I am using 8.
-
- removed comment
Replying to [comment:11 hinder]:
Good point. Yes, the expressions changed between 7 and 8. I don't know about between 8 and 9. I am using 8.
Kranc doesn't currently work with 9.
-
- removed comment
Good to know that Kranc doesn't work with 9. I am struggling to rebuild McLachlan and it is failing with the following messages:
NBT530-403443 20> make rm -rf ML_ADMQuantities* ./runmath.sh McLachlan_ADMQuantities.m Using Kranc installation at ../../../repos/Kranc/Bin/.. Mathematica 9.0 for Linux x86 (64-bit) Copyright 1988-2013 Wolfram Research, Inc. SetDelayed::write: Tag Symmetrize in Symmetrize[x_, i1_, i2_] is Protected. Format::nosym: t:(Tensor[__] | CD[__] | PD[__] | FD[__]).. does not contain a symbol to attach a rule to. Format::nosym: t:(x__) does not contain a symbol to attach a rule to. LinkOpen::linke: Could not find MathLink executable. InstallJava::launch: The Java runtime could not be launched. LinkOpen::linke: Could not find MathLink executable. InstallJava::launch: The Java runtime could not be launched. Part::partw: -- Message text not found -- (2) ({{True, True, True, True, True}}) Last::normal: -- Message text not found -- (1) (Last[True]) Last::normal: -- Message text not found -- (1) (Last[True]) Last::normal: -- Message text not found -- (1) (Last[True]) General::stop: Further output of Last::normal will be suppressed during this calculation. Part::partw: -- Message text not found -- (2) ({{True, True, True, True, True}}) Part::partw: -- Message text not found -- (2) ({{True, True, True, True, True}}) General::stop: Further output of Part::partw will be suppressed during this calculation. Exception: "lookup failure: key Symmetries not found in map {TensorWeight -> 0, Symmetries -> {}}" for thorn in ML_ADMQuantities*; do \ ./copy-if-changed.sh $thorn ../$thorn; \ done make: *** [McLachlan_ADMQuantities.out] Error 2
Would it possible to make Kranc detect the Mathematica version being used and output a clean error message?
Thanks, Bruno.
-
- removed comment
After my previous comment, we merged the necessary changes to Kranc to make it work with Mathematica 9 (see https://github.com/ianhinder/Kranc/pull/92). I don't think this made it in in time for the release, so you will need to use the master branch instead.
-
- removed comment
Are there plans to backport the Kranc change for Mathematica 9 to the release (if it does not break Mathematica 8 compatibility and is a single commit)?
-
- removed comment
Backporting seems like a good idea to me. The changes are not really that dramatic - just renaming a couple of conflicting symbol names. If others agree, then I will backport the commit to the release branch.
-
- removed comment
Can we only backport the change in the Mathematica code and leave the (tested) thorns as they are? Users aren't expected to get 100% the same thorn-code anyway when they run Mathematica on the Mathematica sources.
-
reporter - removed comment
Do the generated thorns change? I'm fine with just backporting the fixes to Kranc.
-
- changed status to resolved
- removed comment
-
- changed status to closed
- edited description
- Log in to comment
Ideally, they should be regenerated whenever Kranc changes, not only before releases.