add slice in Vector.fs and Matrix.fs, add reduceRows and reduceCols in Matrix.fs, add tests by Brulevich-Nikita · Pull Request #18 · Lamagraph/QTreeFSharp

Brulevich-Nikita · 2026-05-08T19:41:30Z

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

…n Matrix.fs, add tests

gsvgit · 2026-05-11T07:40:44Z

+[<Fact>]
+let ``fromCoordinateList with index out of range`` () =
+    let coo =
+        CoordinateList(6UL<nrows>, 6UL<ncols>, [ (9UL<rowindex>, 9UL<colindex>, 13) ])


Ну вообще неплохо бы падать в таком случае. Ну, только не Failwith, а Result.

Понял, буду использовать Result вместо failwith. Тесты переделаю.

gsvgit · 2026-05-11T07:41:53Z

+[<Fact>]
+let ``fromCoordinateList with zero length and some values returns empty matrix`` () =
+    let coo =
+        CoordinateList(


Тоже неплохо сообщать, что точ-то не так.

Извините, не очень понял. Название теста (и соответствующего теста в Векторе) заменю на более информативное.

Я, скорее, про то, что сами по себе такие случаи, кажется, должнв приводить к ошибке, а не "молча" обрабатываться.

gsvgit · 2026-05-11T07:46:15Z

+    let result = toCoordinateList vec
+    Assert.Equal(CoordinateList(7UL<dataLength>, [ (1UL<index>, 100); (3UL<index>, 2); (5UL<index>, 3) ]), result)
+(*
+[<Fact>]


Что не так с этим тестом?

Я написал тест на дубликат индекса, при тестировании выяснил, что поведение в данном случае не определено реализацией. Что лучше сделать: убрать данный тест или добавить обработку дубликатов индексов?

Можно добавить обработку. Можно поступтть как в GraphBLAS: принимать ещё и функцию, которая агрегирует значения с одинаковым индексом.

gsvgit · 2026-05-11T07:48:05Z

+    (colStart: int)
+    (colEnd: int)
+    : Result<SparseMatrix<'a>, string> =
+    match () with


Ээээ. А зачем такой странный match?

gsvgit · 2026-05-11T07:49:39Z

+        let newRows = uint64 (rowEnd - rowStart + 1) * 1UL<nrows>
+        let newCols = uint64 (colEnd - colStart + 1) * 1UL<ncols>
+
+        let coo =


правдв ли нельзя обойтись без конвертации форматов?

Для получения сдвинутых координат я выбрал и правда не самый рациональный подход в виду перевода в coo и обратно, попробую изменить реализацию через рекурсию (и для матрицы, и для вектора)

gsvgit · 2026-05-11T07:54:47Z

+        Ok newMatrix
+
+let reduceRows (op: 'a option -> 'a option -> 'a option) (matrix: SparseMatrix<'a>) : Vector.SparseVector<'a> =
+    let rows = matrix.nrows


Правда ли, что через транспонирование не быстрее?

Не очень понял Ваш вопрос. Если Вы имеете в виду, что одну из функций (например, reduceRows) можно реализовать как есть, а reduceCols через transpose + reduceRows, то она чуть проиграет по производительности, поэтому (на мой взгляд) лучше оставить двумя функциями.

Да, Вы всё правильно поняли. То есть Вы померили и через транспонирование получается медленнее? Тогда ок. ДДОбавьте тольео это в комментрий к коду. Желательно с конкретными цифрами замеров.

gsvgit · 2026-05-11T07:55:07Z

+
+let slice (_start: int) (_end: int) (vector: SparseVector<'a>) : Result<SparseVector<'a>, string> =
+    match () with
+    | _ when _start < 0 -> Error "Start should be >= 0"


Загадочная конструкция.

Старался что здесь, что в Матрице избежать излишних ifов, придерживаясь функционального стиля, поэтому воспользовался такой конструкцией. Подскажите пожалуйста, как сделать лучше: оставить данный match или заменить на if-else? Или реализовать через что-то по типу match (_start < 0, _end < 0, _start > int vector.length - 1, _end > int vector.length - 1, _start > _end) with ...?

if (elif, если надо) --- нормальная конструкция. Пользуйтесь ей.

…andling, slice and more

gsvgit · 2026-05-20T08:09:55Z

+                | false, false ->
+                    Node(emptySub, emptySub, emptySub, insert half (row - halfRow) (col - halfCol) value emptySub)
+
+    let foldQuadtree folder state size tree =


Правда ли, что это не просто общего фида fold и его не надо на верхний уровень вынести?

gsvgit · 2026-05-20T08:10:20Z

+    let mat2Cols = uint64 matrix2.ncols
+    let initialTree = empty newSize
+
+    let foldMatrixB acc rowMat1 colMat1 valMat1 =


Что-то тут сложно происходит. Можете рассказать, как у Вас кронекер работает?

…sts, fix reduces and kronecker

Brulevich-Nikita · 2026-05-28T20:44:20Z

@gsvgit , здравствуйте! Изменение получилось большим, поэтому опишу по пунктам основные изменения, чтобы проще было понимать:

поправил небольшие недочёты
fold и правда общего вида, я вынес его в отдельную функцию и переписал с её использованием reduceRows, reduceCols и Kronecker
Вы спрашивали про принцип работы Кронекера (извините, что сразу не увидел Ваше сообщение). Если кратко, то:
1. создаётся пустое хранилище нужного размера для итоговой матрицы
2. инициализируется функция insert для вставки значения
3. foldMatrixB обходит всё дерево B, применяя для каждого элемента f, вычисляя результат Some computedVal, высчитывает новые координаты элемента и вставляет в итоговое дерево (acc) через insert
4. основной проход по A при помощи foldQuadtree (вынес его в отдельную внешнюю функцию по Вашей рекомендации), для каждого элемента из A вызывает foldMatrixB, передавая текущее итоговое дерево, координаты и значение A
5. Подсчитывает nvals и в случае успеха создаёт SparseMatrix
  Данный способ показался мне наиболее приемлемым (пытался сделать чуть проще - не получалось).
Обернул fromCoordinateList в Option (Result Error / Ok) для матрицы и вектора, из-за этого пришлось переделывать все места во всех файлах QTreeFSharp'а, где использовался fromCoordinateList. Во многих местах из-за этого теряется читаемость и внешний вид, но таким образом валидация данных происходит сразу на уровне координатного списка и избегаются failwith'ы. Подскажите пожалуйста, есть ли у Вас замечания по данному решению?
Исправил свои и чужие failwith'ы, а так же два моих ThrowException, теперь вместо них Result
В данном пункте я могу ошибаться, но хотел бы его с Вами обсудить. Когда я поменял сигнатуру fromCoordinateList'а, пришлось менять все тесты, но изменения, само собой, чисто под сигнатуру, без изменения данных. При запуске все тесты прошли, кроме тестов Борувки. Я посидел с Борувкой и заметил несколько вещей:
1. Тесты, если я не ошибаюсь, вообще проходят мимо, так как в исходном checkResult функция принимает три аргумента "let checkResult name actual expected =", а в каждом тесте два аргумента "checkResult (Graph.Boruvka.mst graph) expected".
2. Когда я это поправил, некоторые тесты продолжили падать из-за своей невалидности. Посмотрите, пожалуйста, как минимум на тест Boruvka MST 10 nodes random.. actual там равно expected на графе с 13 рёбрами и 10 вершинами (что невозможно для Борувки), так к тому же expected содержит явный цикл (0 - 1 - 5 - 6), а изначальный тест горит зелёным.
3. Даже несмотря на пропуск тестов, реализация была не до конца корректной из-за симметричного дублирования рёбер, поэтому я внёс небольшие изменения с каноническим представлением ребра, чтобы алгоритм работал корректно, оставил //комментарии по аналогии с теми, что были.
  Я не до конца могу быть уверен в том, что верно всё понял про Борувку, поэтому, возможно, залез куда не требовалось, но всё же хочу попросить вас посмотреть и проверить, насколько верны мои изменения (как минимум очень смущает случай с якобы проходящим тестом, где в expected циклический граф, когда Борувка должна строить минимальное остовное дерево, что друг другу противоречит), и если был не прав, то откачу изменения назад

Если изменения верные, то начну с замерами.

add slice in Vector.fs and Matrix.fs, add reduceRows and reduceCols i…

f1e8b9c

…n Matrix.fs, add tests

gsvgit reviewed May 11, 2026

View reviewed changes

add Kronecker product to Matrix.fs with tests, fix tests, duplicate h…

c0ba718

…andling, slice and more

gsvgit reviewed May 20, 2026

View reviewed changes

change fromCoordinateList signature, fix failwith, fix boruvka and te…

ad3ccbf

…sts, fix reduces and kronecker

Conversation

Brulevich-Nikita commented May 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Brulevich-Nikita commented May 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants