-
Notifications
You must be signed in to change notification settings - Fork 42
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
Initial implementations of Accelerate backends for Vector and Matrix #39
Conversation
> mmat :: Matrix (UVector "v" Double) Double "a" "b" | ||
> mmat = unsafeToModuleM 2 [0..5] | ||
> | ||
> m :: ACCMatrix Interpreter (ACCVector Interpreter "v" Double ) "a" "b" Double |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would the type Matrix (AccVector Interpreter "v" Double) Double "a" "b"
work?
Awesome work! This is super cool :) I've just briefly glanced through your changes at this point. It's a little bit hard to review them right now though because the changes are spread over multiple commits and intermixed with a bunch of other code. Can you:
for me? |
in ACCVector (A.use arr) | ||
|
||
|
||
-- acc2SVector fails with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You probably need to add Scalar a ~ Scalar (Scalar a)
to the list of constraints for ValidAccVector
CUDA is working; I'm having problems building the acclerate-llvm packages currently so I haven't been able to test those. I'll move everything to an Accelerate directory and see if I can't get the instance methods working and then get back to you. |
initial implementations of Accelerate Vectors are working Algebra.Matrix Remove question about whether Heyting algebras are cancellative semigroups. Also add a comment that they're not cancellative in general, in case the question arises again. (Counterexample: open intervals in R.) Change wording Switch from ^ to && for the subalgebra operation Update README.md finished initial implementation updating packages removing llvm acclerate-llvm is building
Oops! I'm really sorry I didn't see this earlier :( I'm super stoked about this though! It looks awesome! I've merged your code into a separate branch |
Hey, no worries, I was just about to write a note: getting everything working is proving trickier than I thought, there's lots head scratching about getting GHC to infer the correct types under the accelerate types in a given context. (I try one thing, do that all the way through, fail, and then try another). I'm going to get as much as I can working, squash them, and then maybe you can glance over it before I move onto test. (There's a couple instances that throw errors I still don't understand ...). I think probably it's going to take using it a bit in some code to work through some of the missing instances / make sure the computation graphs actually work in non-trivial cases. There's also some other Acclerate api stuff to get in, like streams and flow control with kernel execution, but comes later. |
|
||
|
||
instance (FreeModule r, ValidLogic r, ValidACCVector b n r, IsScalar r) => FiniteModule (A.Exp (ACCVector b (n::Symbol) r)) | ||
instance (KnownNat n, FreeModule r, ValidLogic r, ValidACCVector b n r, IsScalar r) => FiniteModule (ACCVector b (n::Nat) r) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there something defined about FiniteModule such that it requires an Int
? This instance (along with the dim
function below) both yield A.Exp Int
which is the Acclerate scalar int datatype, but I can't seem to make either type check since A.Exp Int is not an Int.
Here's the error:
/Users/timpierson/subhask/src/SubHask/Algebra/Accelerate/Vector.hs: 141, 10
Couldn't match type ‘A.Exp Int’ with ‘Int’
In the instance declaration for ‘FiniteModule (ACCVector b n r)’
/Users/timpierson/subhask/src/SubHask/Algebra/Accelerate/Vector.hs: 141, 10
Couldn't match type ‘A.Exp Int’ with ‘Int’
Inaccessible code in the instance declaration
In the instance declaration for ‘FiniteModule (ACCVector b n r)’
I made a couple inline comments the current major stumbling blocks, if I can get those resolved, I'm hoping there should only be a few more in those instances and getting the dagger type working. Could you take a glance and see if you have any thoughts? I still have some apprehension about nested parallelism and kernel management with the current design. (also we'll have to figure out how to make your test suit call the correct execute functions for these types). |
About About the I just looked at the definition of
So the constraint you really want is |
Just an update, I started to move to ghc 8.0 as I was unable to resolve the |
Okay, thanks for the update. On Sat, Jul 2, 2016 at 9:27 AM, o1lo01ol1o [email protected] wrote:
|
closing this in favor of the ghc 8.0 branch PR. https://github.com/mikeizbicki/subhask/pull/54 |
I've made a first pass through the implementations by copying all or most of the instances for SVector and the newly added Matrix types. There's some problems when the instance methods return Accelerate types (eg, A.Exp) instead of whatever is expected, but I'm not sure how to amend this. There are also a couple of points where some types can't be deduced, maybe you can advise on what's missing there? I've commented out the sections that need to be resolved. When they're working I can finish the tests in the examples folder.
I used the current master branches of the acclerate packages as the hackage packages are now quite old.
The LLVM package wouldn't build when I added it as a dependency, some problem with a missing cabal file, I've opened an issue, maybe it can be fixed. Repa is 5 years without a commit so I left it out in favor of LLVM (which out performed it even 5 years ago).
Let me know how to handle the various issues / what needs to be changed and maybe we can get this finished this week.