-
Notifications
You must be signed in to change notification settings - Fork 3
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
Declutter Internal Shape of Vector #134
Conversation
This commit changes the internal representation of `Vector` to remove tuples on 0- and 1-dimensional vectors. They are now represented as scalars and lists respectively.
Doesn't the other vector types (Push, Manifest, etc) need the same kind of patch? The new type family should morally be closed. What advantage does the closed type family bring if we have to have the open type family around anyways? ExternalProgram is meant as a tool for debugging and testing the compiler so assume that there is an expert user for that. I assume we need some special hardwired knowledge in there to recognise DIM0/DIM1 arrays. |
Yes, I will add instances for The only "advantage" is compatibility with older GHC (pre 7.8) versions. With 7.10 coming out soon, I guess we could remove the open family. Would you be ok with putting |
My question was actually the other way around: if open type families are enough to get the functionality we want, why add extra code for closed type families? Haskell support on RHEL seems to have stopped updating around 7.6 so I'm a bit split about deprecating that right now. Add the cabal flag for now and I'll patch ExternalProgram at a later date. Since this is still hot in your cache: can you please summarise the data structure layout changes from this patch so I don't have to reverse-engineer that when I get to updating ExternalProgram. |
The motivation for the closed family is as you said before, it's morally closed and I wanted to express that to GHC. I guess we could consider any drawbacks of leaving it open.
Only the |
ExternalProgram might actually work as is with that representation. |
It does, see branch Feldspar/feldspar-compiler#197 |
Ok, so can this be merged now? |
Yes, but that will break test-cases in Feldspar/feldspar-compiler. I still have some quirks to sort out there. |
A suggestion wrt to Manifest: the easiest way to deal with Manifest is probably to define it as follows:
|
Declutter Internal Shape of Vector
As a step on the way to implementing #91, this pull request declutters the internal representation of
Pull
.Pull DIM0 a
is represented as a scalarInternal a
.Pull DIM1 a
is represented as a list[Internal a]
.With this change there should be no need for the
Feldspar.SimpleVector
module.However, it breaks some of the assumptions in the
ExternalProgram
module in feldspar-compiler./cc @emilaxelsson, @josefs, @pjonsson