Performance benchmark#6
Conversation
| let (nrows, ncols, nnz) = float matrixInfo.[0], float matrixInfo.[1], float matrixInfo.[2] | ||
| let (vertices, edges) = if nrows = ncols then (nrows, nnz) else (ncols, nrows) | ||
| sprintf "%f" (edges / meanTime) | ||
| | _ -> "file`s format not supported" |
There was a problem hiding this comment.
It would be better to add the current file format to the message. Also, it may be useful to add supported formats.
| abstract Fill: Mask1D option * int -> Scalar<'a> with set | ||
| abstract Fill: int * Mask1D option -> Scalar<'a> with set | ||
|
|
||
| abstract Mxm: Matrix<'b> -> Mask2D option -> Semiring<'a, 'b, 'c> -> Matrix<'c> |
There was a problem hiding this comment.
Hm... What the reason to add two new type parameters to semiring?
|
|
||
| abstract Mxm: Matrix<'b> -> Mask2D option -> Semiring<'a, 'b, 'c> -> Matrix<'c> | ||
| abstract Mxv: Vector<'b> -> Mask1D option -> Semiring<'a, 'b, 'c> -> Vector<'c> | ||
| abstract EWiseAdd: Matrix<'a> -> Mask2D option -> BinaryOp<'a, 'b, 'c> -> Matrix<'a> |
There was a problem hiding this comment.
Why binary operation is not closed? Seems, we had discussed this point. I think semiring should have one type parameter and be closed. If it is really necessary, then one can use tuples of the required size as a domain.
There was a problem hiding this comment.
Yes, it is necessary because there is a parent BFS algorithm, which use non-closed minFirst semiring. The alternative, which use embedding, doesn't seem very intuitive to me. Now, I can make the semiring closed, but this problem will require a more elegant solution in the future.
| abstract EWiseAdd: Vector<'a> -> Mask1D option -> Semiring<'a> -> Vector<'a> | ||
| abstract EWiseMult: Vector<'a> -> Mask1D option -> Semiring<'a> -> Vector<'a> | ||
| abstract Vxm: Matrix<'b> -> Mask1D option -> Semiring<'a, 'b, 'c> -> Vector<'c> | ||
| abstract EWiseAdd: Vector<'a> -> Mask1D option -> BinaryOp<'a, 'b, 'c> -> Vector<'a> |
There was a problem hiding this comment.
Looks like we should use monoid instead of binaryOp, because for sparse operations we need to know an ID. Thus, it may be useful to define semiring as an extension of monoids.
| open GraphBLAS.FSharp | ||
|
|
||
| [<AutoOpen>] | ||
| module SSSP = |
There was a problem hiding this comment.
I think that SSSP should be defined using R^\infinite semiring. Or we assume that the distance between two nodes cannot be 0? If we use R as a domain, then distance can be equals to 0. Thus we should use R^\infinite or R+.
|
|
||
| [<AutoOpen>] | ||
| module TriangleCounting = | ||
| // нужна проекция в инт |
| } | ||
|
|
||
| module BooleanSemiring = | ||
| let anyAll: Semiring<bool> = { |
There was a problem hiding this comment.
Huh... I'm confused. Here we have semiring with one type parameter, but in some files, we use semiring with three type parameters.
|
|
||
| module FloatSemiring = | ||
| let addMult: Semiring<float> = { | ||
| PlusMonoid = FloatMonoid.add |
There was a problem hiding this comment.
Mmm.... Semiring already uses Monoid in definition... Well...
| type Semiring<'a> = { | ||
| PlusMonoid: Monoid<'a> | ||
| Times: BinaryOp<'a, 'a, 'a> | ||
| type Semiring<'T1, 'T2, 'TOut> = { |
| open System | ||
|
|
||
| type VectorType = | ||
| | Sparse = 0 |
There was a problem hiding this comment.
Because they are easier to enumerate. They could be enumerated without reflection.
gsvgit
left a comment
There was a problem hiding this comment.
Looks good enough.
- I think, we should add SSSP over R\infinite semiring in addition to the existing one. Not now, but in the nearest future. It will be useful for DU translation evaluation. It should help to estimate an overhead.
- Build should be fixed.
Proposed Changes
Types of changes
Checklist