Cidr4 merge algorithm #5

Merged
PavelPatsey merged 91 commits from CIDR4_merge_algorithm into main 2025-01-27 22:05:39 +03:00
Showing only changes of commit 1d34eb92bd - Show all commits
+3 -1
View File
@@ -1,6 +1,9 @@
import cProfile import cProfile
Fedor-Lyanguzov commented 2025-01-07 17:54:03 +03:00 (Migrated from github.com)
Review

Здесь за один проход объединяются все возможные объединения, таким образом мы можем промахнуться мимо цели в M элементов списка.

Здесь за один проход объединяются все возможные объединения, таким образом мы можем промахнуться мимо цели в `M` элементов списка.
Fedor-Lyanguzov commented 2025-01-07 17:55:32 +03:00 (Migrated from github.com)
Review

Здесь не хватает возвращения количества адресов, "попавших под раздачу": не принадлежащих начальному списку, но попавших в результат из-за объединения. Это количество позволит найти оптимальное решение.

Здесь не хватает возвращения количества адресов, "попавших под раздачу": не принадлежащих начальному списку, но попавших в результат из-за объединения. Это количество позволит найти оптимальное решение.
Fedor-Lyanguzov commented 2025-01-07 17:57:45 +03:00 (Migrated from github.com)
Review

Я думаю, использование внешней библиотеки (и вообще любой библиотеки) размывает смысл алгоритма: хотя из следующих строк понятно, что будет сделано; однако не очевидно, как это будет сделано, и будет ли оптимальный ответ, и будет ли оптимальное решение (что не обязательно).

Я думаю, использование внешней библиотеки (и вообще любой библиотеки) размывает смысл алгоритма: хотя из следующих строк понятно, что будет сделано; однако не очевидно, как это будет сделано, и будет ли оптимальный ответ, и будет ли оптимальное решение (что не обязательно).
Fedor-Lyanguzov commented 2025-01-12 17:46:01 +03:00 (Migrated from github.com)
Review

Мне кажется, что функция merge_nodes содержит не все свои обязанности, некоторые из них похоже вложись в функцию reduce_nodes. Стоит их переместить.

Мне кажется, что функция `merge_nodes` содержит не все свои обязанности, некоторые из них похоже вложись в функцию `reduce_nodes`. Стоит их переместить.
Fedor-Lyanguzov commented 2025-01-12 19:00:28 +03:00 (Migrated from github.com)
Review

Алгоритм слишком много делает каждый шаг, из-за этого работает медленно. Как мне кажется, для оптимизации стоит разработать алгоритм начиная с рекурсии, возможно их будет штук 5 связанных, зато это позволит определить характеристики отдельных кусков и принять решение по оптимизации.

Алгоритм слишком много делает каждый шаг, из-за этого работает медленно. Как мне кажется, для оптимизации стоит разработать алгоритм начиная с рекурсии, возможно их будет штук 5 связанных, зато это позволит определить характеристики отдельных кусков и принять решение по оптимизации.
Fedor-Lyanguzov commented 2025-01-12 19:01:34 +03:00 (Migrated from github.com)
Review

Пора перенести тесты в отдельный файл?

Пора перенести тесты в отдельный файл?
import sys
from collections import defaultdict from collections import defaultdict
sys.setrecursionlimit(10_000)
Node = tuple[int, int, int, int] Node = tuple[int, int, int, int]
@@ -173,7 +176,6 @@ def merge_nodes_recursion(nodes: list[Node], required_len: int) -> list[Node]:
return nodes return nodes
groups = make_groups(nodes) groups = make_groups(nodes)
neighbours, singles = find_neighbours_singles(groups) neighbours, singles = find_neighbours_singles(groups)
print(f"{len(nodes)=} {len(singles)=} {len(neighbours)=}")
if neighbours: if neighbours:
new_nodes = merge_neighbors(nodes, neighbours) new_nodes = merge_neighbors(nodes, neighbours)
return merge_nodes_recursion(new_nodes, required_len) return merge_nodes_recursion(new_nodes, required_len)