Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add errors and assertions to Acc computations #494

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ivogabe
Copy link
Contributor

@ivogabe ivogabe commented Mar 9, 2021

Description
This pull requests adds throwing functions, similar to error from the Prelude, and assertions to our Acc language.

I added three functions to throw an error:

  • aerror :: Arrays a => String -> Acc a: given a string, throws an error containing that string
  • aerrorArray :: (Arrays a, Arrays b, Show a) => String -> Acc a -> Acc b: takes a string and an array, similar to atraceArray
  • aerrorExp :: (Elt e, Show e, Arrays a) => String -> Exp e -> Acc a: takes a string and an expression, similar to atraceExp

Assertions can be written using assert :: (Assertion a, Arrays bs) => String -> a -> Acc bs -> Acc bs. I'm not sure about its interface yet, so feel free to share some thoughts about that :)

  • Name: We might in the future also add expression-level assertions. It would thus make sense to use aassert for array level assertions, similar to how acond, awhile and atrace are array-level variants of functionalities which are or may be (in the future) available on expressions. Though I didn't like how aassert looked. Hence my idea was to use assert here, and in the future we could use expect for expression-level assertions (note that assert still starts with an a and expect starts with the e of expression, nice coincidence).
  • Predicate: The assertion of course consists of some predicate / condition. My initial thought was to use an Exp Bool, but I also wanted to give better error messages than just saying that the assertion failed. For instance, when asserting that two values are equal I'd like to show those two values. This could be done by making multiple assertion functions, inspecting the expression and detecting several patterns, or having a type class ranging over types of assertions. I didn't really like the first option, that resulted in signatures like assertArraysEqual :: (Shape sh, Eq sh, Elt a, Eq a, Arrays bs) => String -> Acc (Array sh a) -> Acc (Array sh a) -> Acc bs -> Acc bs. With the second approach it would be difficult to detect patterns like array equivalence, as that consists not only of Exp code but also Acc code. I chose the latter. Currently the instances of the type class are Exp Bool (which has no further message), data AssertArraysEqual as = AssertArraysEqual (Acc as) (Acc as) which asserts that arrays are equal, and data AssertEqual e = AssertEqual (Exp e) (Exp e).
  • Array equivalence is currently limited to a single array, we could extend that to tuples of arrays.

Motivation and context
These functions should make it easier for users to detect problems in their code. It is also possible to use these functions for parts of the algorithm which haven't been implemented, similar to undefined from the Prelude, as the program now still compiles and will only crash when such part is actually used.

How has this been tested?
Just a quick test, no automated tests yet.

Types of changes
What types of changes does your code introduce? Put an x in all the boxes that apply:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist
Go over all the following points, and put an x in all the boxes that apply. If you're unsure about any of these, don't hesitate to ask. We're here to help!

  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

I still need to make the accompanying change in accelerate-llvm.

@ivogabe
Copy link
Contributor Author

ivogabe commented Mar 10, 2021

The PR for accelerate-llvm is now open: AccelerateHS/accelerate-llvm#68

@ivogabe
Copy link
Contributor Author

ivogabe commented Mar 10, 2021

@robbert-vdh Could we improve the callstack of those new functions with HasCallStack?

@robbert-vdh
Copy link
Contributor

@ivogabe Yes, it would be possible to add the use site of aerror* to the message! With annotations in place you could just extract that information from the Ann field on the Aerror constructors, but if you want to add that information right now then you could add a SrcLoc field (or an entire CallStack) to constructors and then apply the same call stack tricks I've been using on the branch I've been experimenting with. Let me know if that's what you want and I'll write a patch (of course right now it can be greatly simplified because this information is only needed in these three top level functions).

@tmcdonell
Copy link
Member

One use case which would be good is to add an assertion/error at this point in LULESH (which to now we just silently ignore..): https://github.com/tmcdonell/lulesh-accelerate/blob/master/src/LULESH.hs#L192

@ivogabe
Copy link
Contributor Author

ivogabe commented Mar 10, 2021

We just discussed in our meeting that we should also add Assertion instances for:

  • Acc (Scalar Bool)
  • Acc a -> Exp Bool
  • Acc a -> Acc (Scalar Bool) (this and the previous could perhaps also be a single more generic instance (Assertion a, Show a) => Assertion (Acc a -> a)

The last instance would be useful for the LULESH application for instance, as we could add an assertion assert "All elements should be positive" (all (> 0)) ....

Tom suggested we could also extend the messages by allowing any interleaving of strings and arrays. That would allow us to define those assertions in terms of this more generic one.

@ivogabe
Copy link
Contributor Author

ivogabe commented Mar 15, 2021

I added the option to use functions for assertions, for instance:

condition :: A.Acc (A.Vector Int) -> A.Acc (A.Scalar Bool)
condition = A.all (A.>= 0)

a :: A.Acc (A.Vector Int)
a = A.assert "All elements should be positive" condition $ A.use $ A.fromList (A.Z A.:. 10) [1,2,3,-1,5,6,7,8,9,10]

The downside is that it does require a type annotation, as the compiler cannot resolve the typeclass without it.

I also detected that aerror does not work properly yet. When a worker thread encounters it, it will not crash the entire program, causing that the program gets stuck:

thread blocked indefinitely in an MVar operation

We should abort the entire execution then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants