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 99da7ea726 - Show all commits
+7 -11
View File
@@ -1,9 +1,8 @@
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

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

Пора перенести тесты в отдельный файл?
from collections import defaultdict
from copy import deepcopy from copy import deepcopy
from ipaddress import IPv4Address from ipaddress import IPv4Address
from typing import List, Tuple, Set from typing import List, Set
from itertools import groupby
from collections import defaultdict
import cProfile
def get_data(input_file): def get_data(input_file):
@@ -18,8 +17,7 @@ def cidr4_to_binary(cidr4: str) -> str:
ipv4 = IPv4Address(ip_str) ipv4 = IPv4Address(ip_str)
binary_ip = bin(int(ipv4))[2:] binary_ip = bin(int(ipv4))[2:]
binary_ip = binary_ip.zfill(32) binary_ip = binary_ip.zfill(32)
binary = binary_ip[:vlsm] + "0" * (32 - vlsm) return binary_ip[:vlsm] + "0" * (32 - vlsm)
return binary
def binary_to_cidr4(binary: str) -> str: def binary_to_cidr4(binary: str) -> str:
@@ -30,7 +28,6 @@ def binary_to_cidr4(binary: str) -> str:
def reduce_binary(binary: str) -> str: def reduce_binary(binary: str) -> str:
assert len(binary) == 32
vlsm = binary.rfind("1") vlsm = binary.rfind("1")
if vlsm == -1: if vlsm == -1:
return binary return binary
@@ -43,7 +40,6 @@ def get_mask(binary: str) -> str:
def remove_ips_with_subnets(binaries: Set[str]) -> Set[str]: def remove_ips_with_subnets(binaries: Set[str]) -> Set[str]:
"""Убрать ip, у которых есть подсети"""
sorted_binaries = sorted(binaries) sorted_binaries = sorted(binaries)
i = 0 i = 0
while i < len(sorted_binaries) - 1: while i < len(sorted_binaries) - 1:
@@ -58,7 +54,7 @@ def remove_ips_with_subnets(binaries: Set[str]) -> Set[str]:
def rough_merge_binaries(binaries: List[str], req_len: int) -> List[str]: def rough_merge_binaries(binaries: List[str], req_len: int) -> List[str]:
ips = set(deepcopy(binaries)) ips = set(deepcopy(binaries))
reduction_limit_reached = False reduction_limit_reached = False
max_vlsm = float("inf") max_vlsm = None
while len(ips) > req_len and not reduction_limit_reached and max_vlsm != -1: while len(ips) > req_len and not reduction_limit_reached and max_vlsm != -1:
ip_with_max_vlsm = max(ips, key=lambda x: x.rfind("1")) ip_with_max_vlsm = max(ips, key=lambda x: x.rfind("1"))
max_vlsm = ip_with_max_vlsm.rfind("1") max_vlsm = ip_with_max_vlsm.rfind("1")
@@ -178,5 +174,5 @@ if __name__ == "__main__":
} }
assert remove_ips_with_subnets(set()) == set() assert remove_ips_with_subnets(set()) == set()
main() # main()
# cProfile.run("main()") cProfile.run("main()")