Kranc-generated thorns should be regenerated before the release

Create issue
Issue #1365 closed
Ian Hinder created an issue

Thorns generated by Kranc (McLachlan, WeylScal4 and EinsteinExact) should be generated by the version of Kranc which is being released.

Keyword:

Comments (20)

  1. Frank Löffler
    • removed comment

    Ideally, they should be regenerated whenever Kranc changes, not only before releases.

  2. Barry Wardell
    • 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.

  3. Frank Löffler
    • removed comment

    If we do this it would be good to know what that would actually change for the user.

  4. Erik Schnetter
    • 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.

  5. Frank Löffler
    • 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.

  6. Frank Löffler
    • 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.

  7. Ian Hinder 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)

  8. Barry Wardell
    • 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.

  9. Ian Hinder 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.

  10. Frank Löffler
    • 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.

  11. Ian Hinder 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.

  12. Barry Wardell
    • 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.

  13. Bruno Mundim
    • 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.

  14. Roland Haas
    • 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)?

  15. Barry Wardell
    • 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.

  16. Frank Löffler
    • 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.

  17. Ian Hinder reporter
    • removed comment

    Do the generated thorns change? I'm fine with just backporting the fixes to Kranc.

  18. Log in to comment