add slice in Vector.fs and Matrix.fs, add reduceRows and reduceCols in Matrix.fs, add tests#18
Conversation
…n Matrix.fs, add tests
| [<Fact>] | ||
| let ``fromCoordinateList with index out of range`` () = | ||
| let coo = | ||
| CoordinateList(6UL<nrows>, 6UL<ncols>, [ (9UL<rowindex>, 9UL<colindex>, 13) ]) |
There was a problem hiding this comment.
Ну вообще неплохо бы падать в таком случае. Ну, только не Failwith, а Result.
There was a problem hiding this comment.
Понял, буду использовать Result вместо failwith. Тесты переделаю.
| [<Fact>] | ||
| let ``fromCoordinateList with zero length and some values returns empty matrix`` () = | ||
| let coo = | ||
| CoordinateList( |
There was a problem hiding this comment.
Тоже неплохо сообщать, что точ-то не так.
There was a problem hiding this comment.
Извините, не очень понял. Название теста (и соответствующего теста в Векторе) заменю на более информативное.
There was a problem hiding this comment.
Я, скорее, про то, что сами по себе такие случаи, кажется, должнв приводить к ошибке, а не "молча" обрабатываться.
| let result = toCoordinateList vec | ||
| Assert.Equal(CoordinateList(7UL<dataLength>, [ (1UL<index>, 100); (3UL<index>, 2); (5UL<index>, 3) ]), result) | ||
| (* | ||
| [<Fact>] |
There was a problem hiding this comment.
Я написал тест на дубликат индекса, при тестировании выяснил, что поведение в данном случае не определено реализацией. Что лучше сделать: убрать данный тест или добавить обработку дубликатов индексов?
There was a problem hiding this comment.
Можно добавить обработку. Можно поступтть как в GraphBLAS: принимать ещё и функцию, которая агрегирует значения с одинаковым индексом.
| (colStart: int) | ||
| (colEnd: int) | ||
| : Result<SparseMatrix<'a>, string> = | ||
| match () with |
There was a problem hiding this comment.
Ээээ. А зачем такой странный match?
| let newRows = uint64 (rowEnd - rowStart + 1) * 1UL<nrows> | ||
| let newCols = uint64 (colEnd - colStart + 1) * 1UL<ncols> | ||
|
|
||
| let coo = |
There was a problem hiding this comment.
правдв ли нельзя обойтись без конвертации форматов?
There was a problem hiding this comment.
Для получения сдвинутых координат я выбрал и правда не самый рациональный подход в виду перевода в coo и обратно, попробую изменить реализацию через рекурсию (и для матрицы, и для вектора)
| Ok newMatrix | ||
|
|
||
| let reduceRows (op: 'a option -> 'a option -> 'a option) (matrix: SparseMatrix<'a>) : Vector.SparseVector<'a> = | ||
| let rows = matrix.nrows |
There was a problem hiding this comment.
Правда ли, что через транспонирование не быстрее?
There was a problem hiding this comment.
Не очень понял Ваш вопрос. Если Вы имеете в виду, что одну из функций (например, reduceRows) можно реализовать как есть, а reduceCols через transpose + reduceRows, то она чуть проиграет по производительности, поэтому (на мой взгляд) лучше оставить двумя функциями.
There was a problem hiding this comment.
Да, Вы всё правильно поняли. То есть Вы померили и через транспонирование получается медленнее? Тогда ок. ДДОбавьте тольео это в комментрий к коду. Желательно с конкретными цифрами замеров.
|
|
||
| let slice (_start: int) (_end: int) (vector: SparseVector<'a>) : Result<SparseVector<'a>, string> = | ||
| match () with | ||
| | _ when _start < 0 -> Error "Start should be >= 0" |
There was a problem hiding this comment.
Старался что здесь, что в Матрице избежать излишних ifов, придерживаясь функционального стиля, поэтому воспользовался такой конструкцией. Подскажите пожалуйста, как сделать лучше: оставить данный match или заменить на if-else? Или реализовать через что-то по типу match (_start < 0, _end < 0, _start > int vector.length - 1, _end > int vector.length - 1, _start > _end) with ...?
There was a problem hiding this comment.
if (elif, если надо) --- нормальная конструкция. Пользуйтесь ей.
2 participants
add:
slice in Vector.fs
slice in Matrix.fs
reduceRows in Matrix.fs
reduceCols in Matrix.fs
map in Matrix.fs
(mapi in Matrix.fs and Vector.fs have been already released)
4 map tests in Vector.fs
3 fromCoordinateList tests in Vector.fs
4 mapi tests in Vector.fs
11 slice tests in Vector.fs
6 map tests in Matrix.fs
3 fromCoordinateList tests in Matrix.fs
5 mapi tests in Matrix.fs
21 slice tests in Matrix.fs
7 reduceRows tests in Matrix.fs
7 reduceCols tests in Matrix.fs
to be done:
kronecker with tests