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

Ghc 8.0 accelerate rewrite #54

Open
wants to merge 26 commits into
base: ghc-8.0
Choose a base branch
from

Conversation

o1lo01ol1o
Copy link

With the exception of zero, dim and imap the vector-based instances are compiling in ghc-8.0. I've added them to the test suite but the multiplication test fails with

Couldn't match type ‘Scalar (A.Acc (A.Scalar Int))’
                     with ‘A.Acc (A.Scalar Int)’

This is undoubtedly because I had to define the ValidAcc type with the constraint(s)

, Scalar (A.Acc (A.Scalar a)) ~ A.Acc (A.Scalar a)
, Logic (A.Acc (A.Scalar a)) ~ A.Acc (A.Scalar a)
, Actor (A.Acc (A.Scalar a)) ~ A.Acc (A.Scalar a)

to get the library to compile. I'm not totally sure how to go about refactoring, though, since accelerate will return a A.Acc (A.Scalar a) (this is really an array singleton) for any scalar value resulting from an Array computation. Do you have any suggestions?

Also, I see that Tensor_algebra and a few other instances aren't currently in ghc-8.0. I guess these should just be omitted for now?

o1lo01ol1o and others added 14 commits May 26, 2016 22:11
Merging upstream changes to local master
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

Sorry I didn't see this earlier.

I'm guessing the reason you're getting

Couldn't match type ‘Scalar (A.Acc (A.Scalar Int))’
                     with ‘A.Acc (A.Scalar Int)’

is due to a few lines like:

type instance Scalar (ACCMatrix b v m n r) = Scalar r

Here you are not wrapping the scalar value properly for the accelerate framwork. I think on this line, however, you are wrapping it properly:

type instance Logic (ACCMatrix b v m n r) = A.Acc (A.Scalar Bool)

Does that make sense?

Unfortunately, I can't actually test this theory right now because Github is saying there's a merge conflict. I can take a closer look at the multiplication issue if you resolve the conflict first.

Merged current ghc-8.0 branch
@o1lo01ol1o
Copy link
Author

I've squashed the commits and fixed the conflicts. I think you were seeing some of the earlier commits as I rewrote all the instances to be in terms of A.Acc (A.Scalar r) or the equivalent before I opened this PR.

And I removed the Matrix instances until the Vector instances were working and the rest of the ghc-8.0 branch seemed stable (so you shouldn't even have seen the above mentioned line).

See if the PR state makes more sense now.

@mikeizbicki
Copy link
Owner

Thanks. It'll probably be Thursday/Friday before I get a chance to look at this.

@mikeizbicki
Copy link
Owner

What version of accelerate-cuda are you using? I can't get your branch to compile because the latest version on hackage (0.15.1.1) doesn't support ghc 8.

Just looking through your edits, I think the problem might be with this line:

type instance Scalar (ACCVector bknd n r) = Scalar (A.Acc(A.Scalar r))

AFAICT, you don't define anywhere what Scalar (A.Acc a) should be. One way to fix this would be to just remove the Scalar application from the definition as follows

type instance Scalar (ACCVector bknd n r) = A.Acc(A.Scalar r)

@o1lo01ol1o
Copy link
Author

I'll change that tonight and investigate. There are git commits referenced in the stack config file that should pull the correct combination of packages. It was a little difficult as only certain pulls of the master branch of accelerate compile with the various subpackages (and accelerate-llvm still has no ghc-8.0 candidate, although it looks like some progress is being made towards that end).

@o1lo01ol1o
Copy link
Author

Sorry, I've just been snatching 15 minutes here and there to work on this and I also had the misfortune of needing to update OSX and fix the resulting toolchain breakage.

I reworked the the code and the multiplication error persists. Did you ever manage to build with the acclerate pacakges?

@o1lo01ol1o
Copy link
Author

o1lo01ol1o commented Sep 7, 2016

@mikeizbicki \ping! there's still some issues with this rewrite, the library code compiles but examples usages don't. I really want to see it move along but I don't have the resources to work on it at the moment so the trail is cold, unfortunately.

related:
Have you seen http://www.datahaskell.org/ ? We formed a week ago, there's a fair amount of enthusiasm for work on the datascience ecosystem. If you have the time, your input would be very valuable helping the community understand the state of the datascience problem in haskell. Slack channel: https://datahaskell.slack.com

Examples are calculating but the laws are failing, unsurprisingly.
Conflicts:
	src/SubHask/Algebra/Accelerate/Vector.hs
@o1lo01ol1o
Copy link
Author

Vector now compiles and computes the examples (up until the outerproduct) in examples/example0005-accelerate_backend.lhs. (Using the Interpreter, I haven't checked CUDA yet and won't be able to for a week).

The confusion was that sometimes accelerate returns A.Acc A.Scalar as a scalar and sometimes A.Exp and I was trying to keep everything in Array form. However, you can't lift anything into A.Acc and A.Scalar is actually a singleton. So probably this will have to be refactored in some way to reflect that. The laws are still borked; I just dumped every complaint GHC threw at me into an orphaned instance until I could run the extant calculations.

I'm going to continue with the ExpFields, Squares and Matrix types and then see where everything stands. Maybe I'll look into Reverse AD when I get this done.

Initial rewrite of Vector.hs complete


Merged branch ghc-8.0-accelerate-Acc-Exp into ghc-8.0-accelerate-Acc-rewrite
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