- changed status to open
- removed comment
LSUThorns/Vectors: Update Kranc-specific code
Keyword:
Comments (10)
-
reporter -
- removed comment
I don't quite understand the undefinition of E. If the symbol is used elsewhere with a different meaning, how does that interact with Kranc? Could you elaborate?
-
reporter - removed comment
- define does not have name scopes. If E is defined, we assume it comes from Kranc, and redefine it to its proper value with vectorisation (i.e. with ToReal). If it is not defined, then we don't define it here either.
If we #define E here unconditionally, and some code tries to use it as a variable/type name, then this will fail. This would insert the identifier E into all source code, even those not using Kranc.
-
- removed comment
OK I see. Would it make sense to change the conditional to #ifdef KRANC_C so that you can be sure you are only affecting Kranc thorns? It might be that other header files also define E for their own purposes. For Kranc-generated thorns, we can be fairly sure that this is not the case.
Now that I think about it some more, why is this replacement handled here by macros? Wouldn't it make more sense to have E -> ToReal[E] in Kranc when vectorisation is enabled (and similarly for Pi)? Then Vectors would not need to know about a Kranc-specific symbol, and Kranc would be treating these constants just like it treats parameters (for example).
-
reporter - removed comment
Yes, using KRANC_C would be better.
Yes, doing this in Kranc would also work.
-
- changed status to open
- removed comment
-
- removed comment
So the suggestion is to have something like to two patches I have just attached (one for Vectors, and the other for Kranc)?
-
reporter - removed comment
Yes, similar to these patches.
I plan to commit my new version of the patch (without your patches) later today, and you could apply them then.
-
reporter - removed comment
Applied.
-
- changed status to resolved
- removed comment
- Log in to comment