ianhinder/Kranc

ValueQ change in Mathematica 12.2 breaks Kranc code generation in ET

Closed this issue · 12 comments

Today I tried using the Kranc-based McLachlan Mathematica scripts to regenerate the ML_BSSN thorn in the current stable release of the Einstein Toolkit, and it failed (error below). I'm running Mathematica 12.2.0 on a MacOS machine (10.15.7 -- Catalina). On reporting this to the Einstein Toolkit Users mailing list, and the failure was verified by Roland Haas.

On digging further, Roland found that Mathematica changed the behavior of the "ValueQ" function [ https://reference.wolfram.com/language/ref/ValueQ.html ] between v 12.1 and 12.2. The old behavior can be restored by specifying Method -> "Legacy" when invoking ValueQ in Kranc's TensorTools.m

===================== failure message from make =====================================
gs66-dodder:m bjkelly1$ make McLachlan_BSSN.out
rm -rf ML_BSSN ML_BSSN_Helper
env ML_CODE=BSSN ./runmath.sh McLachlan_BSSN.m McLachlan_BSSN
Using Kranc installation at ../../../repos/Kranc/Bin/..
Mathematica 12.2.0 Kernel for Mac OS X x86 (64-bit)
Copyright 1988-2020 Wolfram Research, Inc.
Profiling disabled

MapThread::mptd: Object TensorTools`Private`TensorCharacter[dir] at position {2, 2} in MapThread[NumberQ[#1] || NumberQ[#2] || #1 === #2 & , {{u}, TensorTools`Private`TensorCharacter[dir]}] has only 0 of required 1 dimensions.

MapThread::mptd: Object TensorTools`Private`TensorCharacter[epsdiss] at position {2, 2} in MapThread[NumberQ[#1] || NumberQ[#2] || #1 === #2 & , {{u}, TensorTools`Private`TensorCharacter[epsdiss]}] has only 0 of required 1 dimensions.

Riffle::list: List expected at position 1 in Riffle[TensorTools`Private`TensorCharacter[gt], ,].

StringJoin::string: String expected at position 1 in StringJoin[Riffle[TensorTools`Private`TensorCharacter[gt], ,]].

StringJoin::string: String expected at position 2 in Tensor indices in gt[la,lb] do not match those used previously: gt[<>Riffle[TensorTools`Private`TensorCharacter[gt], ,]<>].
Error:
StringJoin["Tensor indices in gt[la,lb] do not match those used previously: gt[", Riffle[TensorTools`Private`TensorCharacter[gt], ","], "]"]
make: *** [McLachlan_BSSN.out] Error 1

This patch to Kranc makes it possible to generate ML_BSSN but is not backwards compatible since ValueQ in 12.1 does not take any Method argument.

diff --git a/Tools/CodeGen/TensorTools.m b/Tools/CodeGen/TensorTools.m
index 9e53a0a..1f17b35 100644
--- a/Tools/CodeGen/TensorTools.m
+++ b/Tools/CodeGen/TensorTools.m
@@ -301,7 +301,7 @@ DefineTensor[T_] :=
       Module[
         {c},
         c = tensorCharacter[{is}];
-        If[!ValueQ[TensorCharacter[T]],
+        If[!ValueQ[TensorCharacter[T], Method -> "Legacy"],
            TensorCharacter[T] = c,
            (* else *)
            If[!charactersMatch[c,TensorCharacter[T]],

Documentation for 12.1's ValueQ is here: https://reference.wolfram.com/legacy/language/v12.1/ref/ValueQ.html and indicates that the "old" method of ValueQ was checking of the result of evaluating FOO produced a result that differed from FOO.

You can add this option conditional on the Mathematica version. See, for example, SimulationTools init.m. Something like this might work:

 If[!ValueQ[TensorCharacter[T], If[$VersionNumber >= 12.2, Method -> "Legacy", Sequence[]]],

You can add this option conditional on the Mathematica version. See, for example, SimulationTools init.m. Something like this might work:

 If[!ValueQ[TensorCharacter[T], If[$VersionNumber >= 12.2, Method -> "Legacy", Sequence[]]],

Unfortunately this does not seem to work. In 12.0 this returns ValueQ unevaluated. Eg:

Wolfram Language 12.0.0 Engine for Linux x86 (64-bit)
Copyright 1988-2019 Wolfram Research, Inc.

In[1]:= ValueQ[A, If[$VersionNumber >= 12.2, Method -> "Legacy", Sequence[]]]

Out[1]= ValueQ[A, If[$VersionNumber >= 12.2, Method -> Legacy, Sequence[]]]

In[2]:= If[ValueQ[A, If[$VersionNumber >= 12.2, Method -> "Legacy", Sequence[]]], Print["Value"], Print[
"No value"]]

Out[2]= If[ValueQ[A, If[$VersionNumber >= 12.2, Method -> Legacy, Sequence[]]], Print[Value],

>    Print[No value]]

so the condition would have to be:

If[$VersionNumber >= 12.2, ValueQ[A, Method -> "Legacy"], ValueQ[A]]

everywhere ValueQ occurs.

What works in both versions seems to be to build one's own 12.0 look-alike ValueQ:

SetAttributes[MyValueQ, HoldAll]
MyValueQ[x_] := Not[x === Unevaluated[x]]

T[a] = 1

Print["ValueQ", ValueQ[T[a]], ValueQ[T[b]]]

Print["ValueQ(Legacy)", ValueQ[T[a], Method->"Legacy"], ValueQ[T[b], Method->"Legacy"]]

Print["MyValueQ", MyValueQ[T[a]], MyValueQ[T[b]]]

gives:

In[1]:= SetAttributes[MyValueQ, HoldAll]

In[2]:= MyValueQ[x_] := Not[x === Unevaluated[x]]

In[3]:= T[a] = 1

Out[3]= 1

In[4]:= Print["ValueQ", ValueQ[T[a]], ValueQ[T[b]]]
ValueQTrueTrue

In[5]:= Print["ValueQ(Legacy)", ValueQ[T[a], Method->"Legacy"], ValueQ[T[b], Method->"Legacy"]]
ValueQ(Legacy)TrueFalse

In[6]:= Print["MyValueQ", MyValueQ[T[a]], MyValueQ[T[b]]]
MyValueQTrueFalse

ie MyValueQ reproduces the results of the old ValueQ.

The issue with 12.2 is that ValueQ returns True if any symbol in its argument is defined and in Kranc's case in the query ValueQ[TensorCharacters[T]] that T has a definition present.

You can also work with Ian's original solution but insert and Evaluate in the right place:

ValueQ[A, Evaluate[If[$VersionNumber >= 12.2, Method -> "Legacy", Sequence[]]]]

You can also work with Ian's original solution but insert and Evaluate in the right place:

ValueQ[A, Evaluate[If[$VersionNumber >= 12.2, Method -> "Legacy", Sequence[]]]]

I am not sure about that. If I run that snipped on 12.0 I get:

$ wolfram
Mathematica 12.0.0 Kernel for Linux x86 (64-bit)
Copyright 1988-2019 Wolfram Research, Inc.

In[1]:= ValueQ[A, Evaluate[If[$VersionNumber >= 12.2, Method -> "Legacy", Sequence[]]]]

Out[1]= ValueQ[A, Null]

and

If[ValueQ[A, Evaluate[If[$VersionNumber >= 12.2, Method -> "Legacy", Sequence[]]]], Print["True"], Print["False"]]

returns the If unevaluated. My guess would be that one needs to do a similar trick like you are doing in McLachlan_BSSN.m to swallow the comma.

The next ET release is scheduled to happen in ~3 weeks (end of May). It would of be good to have a fix included in Kranc for it. Is there a chance of the bugfix in #151 (comment) be reviewed and incorporated into Kranc master by then?

It seems like a reasonable fix to me. I think the only questions are:

  1. Does your valueQ always behave exactly like the old ValueQ?
  2. ValueQ appears several times throughout Kranc. Should it be replaced everywhere, or only the one problematic location?

Ah, had some offline discussion with Steve who suggested that maybe this:

SetAttributes[MyValueQ, HoldAll]
MyValueQ[x_] := If[$VersionNumber >= 12.2, ValueQ[x, Method -> "Legacy"], ValueQ[x]]

since that will put the burden of "works as before" on MMA's "Legacy" method.

I thought about changing it everywhere just to be safe. When I looked into that i could not immediately find a good place for where to define MyValueQ so that it is defined in only a single file yet accessible everywhere. Is there file that all Kranc source files include? Somehting like "KrancDefinitions.m"?

Hi, I was also looking into this issue this morning, but then was AFK for the afternoon so missed all the above!

  • I came up with
SetAttributes[ValueQLegacy, HoldAll];
ValueQLegacy[x_] :=
  ValueQ[x, Evaluate[If[$VersionNumber >= 12.2, Method -> "Legacy", Unevaluated@Sequence[]]]]

where the "Unevaluated" is the missing piece in Barry's solution, and then to use ValueQLegacy wherever ValueQ is used throughout Kranc.

  • But Steve's solution is the best! I think ValueQLegacy is a better name than MyValueQ. So I would go with Steve's solution.
  • I think you can put it in Kranc.m; this should be used by most of the packages, and there's already some Mathematica version compatibility code in there.

I am in the process of upgrading Mathematica to 12.2.0 for the purpose of implementing and testing this for McLachlan, on both 12.0.0 and 12.2.0, but I'm totally overloaded at the moment, so if there is someone who could do this instead, that would be great! I'm happy to review a PR.

here's a pull request: #152

Tested that with the pull request and MMA 12.2.3 one can generate McLachlan master.

The pull request introduces ValueQLegacy in Kranc.m and uses it everywhere except RunKranc.m.