McLachlan should not define one-character variables

Issue #1233 new
Frank Löffler created an issue

McLachlan defines several one-character grid functions, making it impossible to use these in thorns inheriting from McLachlan. One example is 'A' which in Fortran is the same as 'a' which is often used as temporary variable. 'A' in McLachlan is the time derivative of the lapse, and in the ML_dtlapse group. Would there be any problem renaming the grid function also to ML_dtlapse? (and similarly others, e.g., B and H and possible some of the two-character variables as well)

Keyword:

Comments (2)

  1. Ian Hinder
    • removed comment

    I do not like this solution. I think the code should be easily readable by someone who is familiar with the problem domain; i.e. the BSSN equations. In that domain, quantities like H, K, A, etc are understood to have certain meanings. Name conflicts are usually handled by some namespace feature. Your suggestion is a low-tech version of this. I worry that the code would rapidly become unreadable and much longer. Taking your suggestion to its logical conclusion, no thorn which ever expects to be inherited from could use simple, easy-to-understand, names for its grid functions. If a thorn chooses to inherit from another thorn, it should make sure not to use variables which are defined by the public interface of that thorn.

    Why do you say it is impossible to use single-character variables in thorns inheriting from McLachlan? You just need to avoid shadowing the variables by choosing your temporary variable names so that they don't conflict. I assume the compiler warns about shadowed variable declarations, in which case you can rename your temporary variable when you see that warning.

    One practical problem with changing the variable names is that existing parameter files which list these variables by name for output would become invalid, and this is always frustrating.

  2. Frank Löffler reporter
    • marked as
    • removed comment

    In my case I now had to adapt an existing thorn which I wanted to have access to some (but not all) McLachlan variables. This is now done, but the general problem remains.

    Luckly McLachlan doesn't define K, as k is an often used temporary variable name - exactly what I had in my case, just with 'A' and 'B'.

    I agree that longer names can be cumbersome. The would definitely make expressions 'longer'. Also - we might actually 'get away' with most of this once the READS/WRITES mechanism is implemented fully and not everything is imported through inheritance automatically.

  3. Log in to comment