accelerate
accelerate copied to clipboard
Examples of incorrect output from pretty printer
Description This bug contains some examples where the pretty printer currently generates incorrect or ambiguous results. Please feel free to add new examples as you find them!
Steps to reproduce
Load up the example(s) and show
them.
nested :: Exp (Maybe Bool) -> Exp Int
nested = match \case
Nothing_ -> 0
Just_ False_ -> 1
Just_ True_ -> 2
gives (with a sufficiently narrow terminal to not put it on one line):
\(x0, T1 (x1, ())) -> -- here
case x0 of
0 -> 0
1 -> case x1 of
0 -> 1
1 -> 2
EDIT: This has both T1
(even though it is a pair, so should be T2?) as well as pretty printing as a pair itself (,)
; it should be one or the other.
map (\a -> let b = a * a ; c = b * b in T2 c (T3 a b c)) (use (fromList Z [1 :: Float]))
show
s as:
map (\x0 -> let x1 = x0 * x0 x2 = x1 * x1 in T2 x2 (T3 x0 x1 x2)) (use (Scalar Z [1.0]))
which needs some ;
between the let
bindings. If the let-bindings get sufficiently wide that they are split over multiple lines, all is fine again.
Also @tmcdonell I'm not sure what's going wrong in your example; is it the T1
?
@tomsmeding Yes, it's the T1
(edited the description)
EDIT: please see my next message for a smaller repro
This one I haven't been able to reproduce with fusion-processed programs, only on the so-called "internal AST". Therefore, a ghci session:
$ stack repl
...> :m *Data.Array.Accelerate.Trafo
*Data.Array.Accelerate.Trafo> import qualified Data.Array.Accelerate as A
*Data.Array.Accelerate.Trafo A>
Then:
let singleton x = A.use (A.fromList A.Z [x :: Float])
in Sharing.convertAccWith defaultOptions
(let A.T2 a1 (A.T2 a2 (A.T2 a3 _)) = A.T2 (singleton 1) (A.T2 (singleton 2) (A.T2 (singleton 3) (A.lift ())))
in A.zipWith (*) a3 a2)
show
s as:
let
T2 a0 (T2 a1 ()) =
let
T2 a0 (T2 a1 (T2 a2 ())) =
let () = () in T2 (use (Scalar Z [1.0])) T2 (use (Scalar Z [2.0])) T2 (use (Scalar Z [3.0])) ()
in
T2 a1 T2 a2 () -- here
in
zipWith (\x0 x1 -> x0 * x1) a1 a0
which contains T2 a1 T2 a2 ()
, which is incorrect. This example is probably reducible, but at least this does show it (it seems to be rare somehow?).
EDIT: The line starting with let () = ()
also has parenthesis-less nested T2
:)
Smaller reproducing example for the above missing-parentheses issue:
T2 (T2 (lift ()) (use (fromList Z [1]))) (use (fromList Z [2])) :: Acc (((), Scalar Float), Scalar Float)
show
s as:
T2 T2 () (use (Scalar Z [1.0])) (use (Scalar Z [2.0]))
Not sure why I was unable to find this one before.
Can I petition for a mode in the pretty-printer that attempts to output valid Haskell input code? Having to manually translate a zillion occurrences of Tn
to In
is... time-consuming.
I think that will be very hard or impossible, as we only have representation types and thus cannot distinguish tuples and shapes.
A related request, can we have globally unique variable names? That'll make it easier to reason about programs. Currently variables can occur in the left hand side of a let-binding and in its binding, as they have different scopes.
By having a separate counter in the pretty printer, to denote the next fresh name, we could have globally unique names.
Another case where putting multiple things on a single line produces ambiguity: (EDIT: actually, more likely to be a precedence issue somewhere)
let a = use (fromList (Z :. (1::Int)) [1.0::Float]) in reshape (I1 (1 - (let I1 x = shape a in x) - (let I1 x = shape a in x))) a
show
s as:
let a0 = use (Vector (Z :. 1) [1.0])
in
reshape (1 - let T1 x0 = shape a0 in x0 - let T1 x0 = shape a0 in x0) a0
where the 1 - let A = B in C - let D = E in F
part should be 1 - (let A = B in C) - let D = E in F
, because otherwise the body of the first let
would also contain the second let
, which makes the -
's associate incorrectly.
This bit me when trying to feed pretty-printed output back into Accelerate :)
Pretty-printing a single-element tuple is inconsistent between left-hand sides and right-hand sides. This is due to the special cases in prettyAtuple and prettyTuple which are not treated specially in prettyLhs.
This results in the situation where the following program:
map (\(I1 x) -> I1 x) (map (\x -> I1 x) (use (fromList (Z :. (1 :: Int)) [1::Int])))
is show
n as:
let
a0 = use (Vector (Z :. 1) [1])
a1 = map (\x0 -> x0) a0
in
map (\(T1 x0) -> x0) a1
which is type-incorrect as shown, except if you know that both x0
expressions are really T1 x0
in the AST.
I petition to change the case for expressions to explicitly write T1
as well (i.e. to just remove the linked lines in pretty{At,T}uple
). An alternative would be to introduce the special case for left-hand sides as well, but I think less ambiguity is better (and currently for expressions and array programs, a
is ambiguous between the forms a
and ((), a)
in the AST).
(EDIT: on an unrelated note, why are those maps not fused? Avoiding to fuse maps that don't do work striking again?)
- I think you are correct in that removing ambiguity is probably better, so let's remove the special cases in the linked lines.
- yes that should just be manipulating the struct-of-array representation so is silently avoiding fusion (and hoping that the backend does the intended thing)