Repository navigation
The Bundle datatype seemingly doesn't need the field sVector #269
Description
Activity
- changed the title
[-]`Bundle` seemingly doesn't need the field `sVector`[/-][+]The Bundle datatype seemingly doesn't need the field sVector[/+]on Jan 26, 2020 - Good question! I’m not sure offhand, I’ll take a look, and we should definitely look at the history and applicable design writings. Also do any producers generate a Just v ?! There’s absolutely a lot of ways we can improve vector and I welcome meticulous experimentation…On Sun, Jan 26, 2020 at 12:20 AM Genki Sato ***@***.***> wrote: In the package Data.Vector.Fusion.Bundle.Monadic, the datatype Bundle m v a has the field sVector :: Maybe (v a), whose value is currently used by no consumer. I am just curious how come it's there. Is there any practical or historical reason? Is it even OK to make a pull request in favor of the removal of that field? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#269?email_source=notifications&email_token=AAABBQVR4GKNRQXNDQ3CTSDQ7UMTLA5CNFSM4KLU5R72YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4IIXL54A>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAABBQRHEEOWZVUAYS6C54DQ7UMTLANCNFSM4KLU5R7Q> .
At least one generator does generate
Justv:Data.Vector.Fusion.Bundle.Monadic.fromVector(and hence so doData.Vector.Generic.streamandData.Vector.Fusion.Bundle.fromVector). Also,Data.Vector.Fusion.Bundle.Monadic.transpreserves the value of the field. If I've read through and searched within the code correctly, they are the only functions that can giveBundlehavingJustinsVector.Oops. There's
Data.Vector.Generic.stream', of course! ;)all good :)
I don't understand why this has got closed. Could you explain why? My point is that someone assigns it, but nobody uses it. Maybe for use from other package?
- You indicated that you misread the code as dead? Or Did I misinterpret your remark ? I’ve been juggling a lot today so could totally be a mistake…On Thu, Jan 30, 2020 at 10:45 PM Genki Sato ***@***.***> wrote: I don't understand why this get closed. Could you explain why? — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#269?email_source=notifications&email_token=AAABBQWANR23G6NDBJ5WN6DRAONFZA5CNFSM4KLU5R72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKNMLVY#issuecomment-580568535>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAABBQRPBWAVMSODSCOU7KTRAONFZANCNFSM4KLU5R7Q> .
My point is: some agents (
Generic.stream,Generic.stream',Bundle.Monadic.fromVectorandBundle.fromVector) give a value to it, other (Bundle.Monadic.transandBundle.lift) leave the value in it as it is, but if I eye-grepped right, nobody takes a value out of it.- Ohhhh ok. There’s certainly some room for improvement…On Thu, Jan 30, 2020 at 11:20 PM Genki Sato ***@***.***> wrote: My point is: some agents (Generic.stream, Generic.stream', Bundle.Monadic.fromVector and Bundle.fromVector) give a value to it, other (Bundle.Monadic.trans and Bundle.lift) leave the value in it as it is, but if I eye-grepped right, nobody takes a value *out* of it. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#269?email_source=notifications&email_token=AAABBQXGGLXZDWFPDXSTKDLRAORHPA5CNFSM4KLU5R72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKNNY7Y#issuecomment-580574335>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAABBQXHHKJOKAXHLZTZVETRAORHPANCNFSM4KLU5R7Q> .
There are some options I can think of:
1: We could add some consumption: unstreaming operation, slicing operation, and indexing may take the vector out ofsVector. There may or may not be performance penalty. I'm pretty doubtful there will be performance benefit. (I say "doubtful"; I didn't test.)
2: We could entirely removesVector. May get a slight performance benefit?
3: Leave it as it is. I don't say this is a very good choice. However it's not an utterly bad option, if you think ofData.Vector.Fusionas non-internal. In fact, I found this issue when I was writing Stream Fusion code outside of vector.
4: I am wrong. My eye-grep is buggy and there's an usage I didn't find, or the field is some mysterious magic getting the whole thing blazing fast ;)The whole reason why
Bundlewas created is to utilize SIMD instructions. It is described in this paper: https://www.microsoft.com/en-us/research/wp-content/uploads/2016/07/haskell-beats-C.pdf
At some point, I think, there was a fork of vector that worked with some fork of GHC that was actually able to utilize CPU vectorization, but currently this is not the case and I believesVectorisn't used for anything constructiveSo, what do we do about it?
- We could put effort into removing it, but I highly doubt that we get any performance benefit out of it, maybe only compilation times will improve. It is important to realize, however, that there is a huge danger of introducing new bugs in undertaking like this.
- We could leave it as is and wait until ghc acquires native SIMD support, and try to finish what the research in the paper has started
I am leaning towards the latter.
CC @Shimuuar This is my opinion on this issue, since it came up in the release plan.
Linking #251 as related ticket on the subject.
Oh, yeah, I forgot that we found a use for
sVector😉So, let's keep this issue open, but without any rush of removing
sVectorand Bundle restructure, who knows what other uses we can find for it.Let's keep
sVectorthen.Reacted by Alexey Kuleshevich- I remember @dolio found that the bundle fusion layer actually helped more things fuse. But I don’t remember the specific examples.…On Sun, Jan 17, 2021 at 12:32 PM Bodigrim ***@***.***> wrote: Let's keep sVector then. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#269 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAABBQQIUSGESSSEK5XRGCTS2MNKPANCNFSM4KLU5R7Q> .Reacted by Alexey Kuleshevich
In the module
Data.Vector.Fusion.Bundle.Monadic, the datatypeBundle m v ahas the fieldsVector :: Maybe (v a), whose value is currently used by no consumer, apparently (by which I mean I didn't find any usage when I scanned through the code). I am just curious how come it's there. For what (practical or historical) reason does the field exist? Is it even OK to make a pull request in favor of its removal?