Split #75
Split #75
Conversation
| <Compile Include="Vector\Dense\Vector.fs" /> | ||
| <Compile Include="Vector/Sparse/Common.fs" /> | ||
| <Compile Include="Vector/Sparse/Map2AtLeastOne.fs" /> | ||
| <Compile Include="Vector/Sparse/Map2.fs" /> |
README.md
Outdated
| - [ ] `iter` | ||
| - [ ] ... | ||
| - **Vector** | ||
| - [ ] `map` | ||
| - [x] `map` | ||
| - [ ] `iter` | ||
| - [ ] `filter` |
README.md
Outdated
| - [ ] CSR-CSR multiplication | ||
| - [ ] COO transpose | ||
| - [ ] CSR transpose | ||
| - [x] COO-COO `map2` |
There was a problem hiding this comment.
I don't quite understand, masking is expressed through map2. Should I explicitly state this?
There was a problem hiding this comment.
Hmmm... I think, you are right. I guess, we should provide mapping between GraphBLAS# and GraphBLAS API.
| @@ -470,3 +471,232 @@ module ClArray = | |||
| assignValues processor firstValues secondValues positions result | |||
|
|
|||
| result | |||
|
|
|||
| let getChunk (clContext: ClContext) workGroupSize = | |||
There was a problem hiding this comment.
Can it be unified with Array.Copy?
| |> Seq.map (fun lazyValue -> lazyValue.Value) | ||
| |> Seq.toArray | ||
|
|
||
| let blit<'a> (clContext: ClContext) workGroupSize = |
There was a problem hiding this comment.
I think it's still a blit
There was a problem hiding this comment.
Well. I was confused by differences between .net and f# libraries. For our library f# stile is preferred.
| if firstPosition + count > targetArray.Length then | ||
| failwith "The array should fit completely" | ||
|
|
||
| if firstPosition < 0 then |
There was a problem hiding this comment.
Why we can not require firstPosition ti be uint? The same for other parameters that must be nonnegative.
There was a problem hiding this comment.
The original api uses ints...
|
|
||
| processor.Post(Msg.CreateRunMsg<_, _>(kernel)) | ||
|
|
||
| let pairwise (clContext: ClContext) workGroupSize = |
There was a problem hiding this comment.
I suppose it's still pairwise
| let copy = ClArray.copy clContext workGroupSize | ||
|
|
||
| let copyData = ClArray.copy clContext workGroupSize | ||
|
|
||
| let vectorCopy = | ||
| Vector.Sparse.Vector.copy clContext workGroupSize |
There was a problem hiding this comment.
Vector.Sparse.Vector looks strange. Can one of Vector-s be removed?
| open GraphBLAS.FSharp.Backend.Objects.ClVector | ||
| open GraphBLAS.FSharp.Backend.Objects.ClMatrix | ||
|
|
||
| type Indices = ClArray<int> |
There was a problem hiding this comment.
If it is really useful, can it be used everywhere we use indices?
But I'm quite sceptical about type aliases in F# because one still can pass ClArray as a parameter. So, type aliases are not restrictive enough to be useful.
| let map2 = | ||
| map2Inplace<'a, 'b, 'c> clContext workGroupSize map | ||
| map2InPlace<'a, 'b, 'c> map clContext workGroupSize |
Proposed Changes
Types of changes
What types of changes does your code introduce to GraphBLAS-sharp?
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...