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

Initial implementations of Accelerate backends for Vector and Matrix #39

Closed
wants to merge 10 commits into from

Conversation

o1lo01ol1o
Copy link

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.

> mmat :: Matrix (UVector "v" Double) Double "a" "b"
> mmat = unsafeToModuleM 2 [0..5]
>
> m :: ACCMatrix Interpreter (ACCVector Interpreter "v" Double ) "a" "b" Double
Copy link
Owner

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?

@mikeizbicki
Copy link
Owner

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:

  1. move all of the Accelerate specific code into a new Accelerate folder under the Algebra folder; it'd be good to create separate Vector.hs and Matrix.hs and whatever other files in there
  2. squash the commits

for me?

in ACCVector (A.use arr)


-- acc2SVector fails with:
Copy link
Owner

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

@o1lo01ol1o
Copy link
Author

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
@mikeizbicki
Copy link
Owner

mikeizbicki commented Jun 21, 2016

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 o1lo01ol1o-tpierson_gpu_vec for now. There were a few minor edits needed to make it compatible with master, so try to work from the modified code. The last step to get this merged into master is adding to the tests suite. See, for example, how the other vectors are tested: https://github.com/mikeizbicki/subhask/blob/master/test/TestSuite.hs#L43 and just replicate that. I don't think you should have to modify any other files.

@o1lo01ol1o
Copy link
Author

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)
Copy link
Author

@o1lo01ol1o o1lo01ol1o Jun 22, 2016

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)’

@o1lo01ol1o
Copy link
Author

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).

@mikeizbicki
Copy link
Owner

About FiniteModule and Int: No, there's nothing inherent about the index needing to be an Int, it's just traditional. It'd be theoretically just fine to remove that constraint. This might make type inference a bit more of a pain in some circumstances, but I think that's a fine tradeoff for now. The same is true for the return type of dim. So my advice here is to modify the class constraints however is necessary to get everything to type check.

About the Bool problem: This probably stems from a Logic a ~ Bool constraint somewhere (or equivalently a ClassicalLogic a constraint). There's two ways these constraints come up. First, when using types defined in the standard Prelude, they use boolean logics. Second, GHC 7.10's type checker isn't powerful enough to handle a proper equality hierarchy, and I occassionally added these constraints as a workaround. There's currently a branch for GHC 8.0 that has a better story with the numeric hierarchy (specifically because of the UndecidableSuperClasses extension). Upgrading to that branch might fix the problem.

I just looked at the definition of ValidACCVector and found an Ord constraint. In the master branch, it is defined as

type Ord a = (Ord_ a, ClassicalLogic a)

So the constraint you really want is Ord_. This is one of the warts I mentioned about the class hierarchies in master.

@o1lo01ol1o
Copy link
Author

Just an update, I started to move to ghc 8.0 as I was unable to resolve the Bool constraint problem but have run into some dependency snags with accelerate. I've also become swamped with work so will only be making incremental progress until things lighten up. I'll post back when i have some updates.

@mikeizbicki
Copy link
Owner

Okay, thanks for the update.

On Sat, Jul 2, 2016 at 9:27 AM, o1lo01ol1o [email protected] wrote:

Just an update, I started to move to ghc 8.0 as I was unable to resolve
the Bool constraint problem but have run into some dependency snags with
accelerate. I've also become swamped with work so will only be making
incremental progress until things lighten up. I'll post back when i have
some updates.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#39 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABAP1pzEN2sm1-6W2pfjIU3KKVm02xNCks5qRpGPgaJpZM4IxnJS
.

@o1lo01ol1o
Copy link
Author

closing this in favor of the ghc 8.0 branch PR. https://github.com/mikeizbicki/subhask/pull/54

@o1lo01ol1o o1lo01ol1o closed this Jul 27, 2016
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.

2 participants