mikeizbicki / subhask

Type safe interface for working in subcategories of Hask
BSD 3-Clause "New" or "Revised" License
418 stars 43 forks source link

Ghc 8.0 accelerate rewrite #54

Open o1lo01ol1o opened 8 years ago

o1lo01ol1o commented 8 years ago

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?

mikeizbicki commented 8 years ago

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.

o1lo01ol1o commented 8 years ago

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 commented 8 years ago

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

mikeizbicki commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

@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

o1lo01ol1o commented 8 years ago

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.