Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Codereview #4

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
.idea/
**__pycache__/
**.out
**.in
File renamed without changes.
1 change: 1 addition & 0 deletions graphs/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
__author__ = 'Антон Брагин'
53 changes: 53 additions & 0 deletions graphs/bellman_ford.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
__author__ = 'Антон Брагин'

class Graph:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Для таких задач класс это overkill


def __init__(self, n, edges, total_weight):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Насколько я понимаю, total_weight это sum(w for _, _, w in edges). Поэтому лучше не передавать его в качестве параметра, а вычислять при инициализации, иначе можно передать неправильный total_weight.

"""
Create graph representation suitable for Bellman-Ford traversal

:param n: number of vertices
:param edges: tuples of form (edge source, edge sink, edge weight)
:param total_weight: total weight of edges in the graph (assuming they all are non negative)
"""
self.n = n
self.edges = edges
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

edges это список, он изменяемый, поэтому лучше его скопировать, иначе можно сделать так:

g = Graph(edges)

edges[0] = 42 # и сломался g

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self.edges = edges[:]

#In graph with positive edge weights path longer then total_weight + 1 can't exist
self.distances = [total_weight + 1 for i in range(n)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

используй _ вместо i для переменных, которые не нужны

[total_weight + 1 for _ in range(n)]


def find_distance(self, start, finish):
self.distances[start - 1] = 0

for i in range(1, self.n):
for u, v, w in self.edges:
if self.distances[v] > self.distances[u] + w:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

этот if проще записать как

self.distances[v] = min(self.distances[v], self.distances[u] + w)

self.distances[v] = self.distances[u] + w

return self.distances[finish - 1]

def parse_input(file):
with open(file) as fin:
n, s, f = [int(x) for x in fin.readline().split()]
total_weight = 0

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

странное использование пустых строк.

edges = []

for u in range(n):
adjacent = [int(x) for x in fin.readline().split()]
for v, w in enumerate(adjacent):
if w != -1 and u != v:
edges.append((u, v, w))
total_weight += w

return n, edges, s, f, total_weight

#Execution
n, edges, s, f, total_weight = parse_input('pathmgep.in')
graph = Graph(n, edges, total_weight)
d = graph.find_distance(s, f)

with open('pathmgep.out', 'w') as fout:
if d <= total_weight:
fout.write(str(d))
else:
fout.write(str(-1))
96 changes: 96 additions & 0 deletions graphs/components.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
__author__ = 'Антон Брагин'

import logging
import sys
from collections import deque

sys.setrecursionlimit(200000)

logger = logging.getLogger()
logger.setLevel(logging.INFO)
logger.addHandler(logging.StreamHandler(sys.stdout))

class Graph:
def __init__(self, n, m):
self.n = n
self.m = m
self.vertices = [[] for i in range(n)]
self.component = [0 for i in range(n)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[0] * n

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

И я бы инициализировал Noneами, а не нулём -- чтобы было совсем понятно, что компоненты нет


def __str__(self):
str_repr = 'Vertices: ' + str(self.n) + ' Edges: ' + str(self.m) + '\n'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

str_repr = 'Vertices: {} Edges: {}\n'.format(self.n, self.m)

for i, v in enumerate(self.vertices):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

лучше этот цикл написать в виде

str_repr += '\n'.join('bla-bla-bla'.format(...) for i, v in enumerate(self.vertices))

str_repr = str_repr + '{} (comp {}) is connected to: {}'.format(str(i), self.component[i], str(v)) + '\n'
return str_repr

def create_graph(file_in):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Раз уж есть класс, то было бы логичнее сделать

class Graph:
    @staticmethod
    def from_file(f_name):
        ...

with open(file_in) as f:
nm = [int(x) for x in f.readline().split()]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

n, m = [int(x) for ...]


graph = Graph(nm[0], nm[1])
for line in f:
if len(line.strip()) > 0:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if line.strip():
    ...

vx = [int(x) for x in line.split()]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

и тут тоже v, x =

#Self connections does not affects components
if vx[0] != vx[1]:
graph.vertices[vx[0] - 1].append(vx[1] - 1)
graph.vertices[vx[1] - 1].append(vx[0] - 1)

return graph

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Все функции дальше должны быть методами

def get_components_dfs(graph):
component = 0
for i in range(graph.n):
if graph.component[i] == 0:
component = component + 1
dfs(graph, i, component)

return component

def dfs(graph, i, component):
logger.debug('Starting DFS from {}'.format(i))
graph.component[i] = component
for linked in graph.vertices[i]:
if linked != i and graph.component[linked] == 0:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

linked != i дважды лишняя проверка. Первый раз она лишняя, потому что при создании графа ты проверяешь на петли явно, второй раз одна лишняя, потому что ``graph.component[i] != 0:

dfs(graph, linked, component)

def get_components_bfs(graph):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Эта функция отличается от get_components_dfs одной буквой. Можно дедуплицировать:

def get_components(graph, traversal=dfs):
     component = 0
     for i in range(graph.n):
         if graph.component[i] == 0:
             component = component + 1
             traversal(graph, i, component)

component = 0
for i in range(graph.n):
if graph.component[i] == 0:
component = component + 1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

component += 1

bfs(graph, i, component)

return component

def bfs(graph, i, component):
logger.debug('Starting BFS from {}'.format(i))

vertices_deque = deque()
vertices_deque.append(i)

while len(vertices_deque) > 0:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while vertices_deque:

logger.debug('Deque: {}'.format(vertices_deque))
current = vertices_deque.popleft()

graph.component[current] = component

for linked in graph.vertices[current]:
if linked != current and graph.component[linked] == 0:
vertices_deque.append(linked)

def print_components(file_out, graph, ncomp):
with open(file_out, 'w') as f:
f.write(str(ncomp))
f.write('\n')
for vc in graph.component:
f.write(str(vc))
f.write(' ')

##Execute script
graph = create_graph('components.in')
logger.debug('Graph created: {}'.format(graph))
ncomp = get_components_bfs(graph)
logger.debug('Graph analyzed: {}'.format(graph))

print_components('components.out', graph, ncomp)
145 changes: 145 additions & 0 deletions graphs/components_fast.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
__author__ = 'Антон Брагин'

import sys
from collections import deque

#Set it if you are planning to use DFS
sys.setrecursionlimit(200000)

#This is C-style graph representation that is extremely unnatural for Python
#But I wasn't able to found object-based solution that could pass time tests
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вообще это странно, так не должно быть =)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ога, разобрался, почему тормозит =)

Могу рассказать, или можешь сам найти bottleneck в graphs/components.py. Возможно тебе поможет https://docs.python.org/3.4/library/profile.html.

Рассказывать?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Пока не нужно, спасибо, попробую разобраться сам)

class Graph:

def __init__(self, vertices, edges):
"""
Initialize graph object
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Коменты, дословно повторяющие код не нужны.


:param vertices: number of vertices in the graph (nonzero by definition)
:param edges: number of edges in the graph
"""
self.vertices = vertices
self.head = [-1 for i in range(vertices)]
self.component = [-1 for i in range(vertices)]
self.next = [-1 for i in range(2 * edges)]
self.edge = [-1 for i in range(2 * edges)]

self.edge_counter = 0

def addEdge(self, v, w):
"""
Add edge to graph

:param v: edge source
:param w: edge sink
:return: void
"""
#Skip self-edges
if (v == w):
return
#Convert to zero based
v -= 1
w -= 1

self.edge[self.edge_counter] = w
self.next[self.edge_counter] = self.head[v]
self.head[v] = self.edge_counter
self.edge_counter += 1

self.edge[self.edge_counter] = v
self.next[self.edge_counter] = self.head[w]
self.head[w] = self.edge_counter
self.edge_counter += 1

def dfs(self, v, component):
"""
Run DFS from the vertex specified and mark components

:param v: vertex to start from
:param component: current component
"""
self.component[v] = component

e = self.head[v]
while e >= 0:
w = self.edge[e]

if self.component[w] == -1:
self.dfs(w, component)

e = self.next[e]

def bfs(self, v, component):
"""
Run BFS from the vertex specified and mark components

:param v: vertex to start from
:param component: current component
"""
vertices_deque = deque()
vertices_deque.append(v)

while len(vertices_deque) > 0:
current = vertices_deque.popleft()
self.component[current] = component

e = self.head[current]
while e >= 0:
w = self.edge[e]

if self.component[w] == -1:
vertices_deque.append(w)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Хм, а не может ли тут получится, что одна вершинка добавляется в deque несколько раз?


e = self.next[e]

def create_graph(file_in):
"""
Create grapth from the file provided

:param file_in: file to read graph from
:return: Graph object
"""
with open(file_in) as f:
nm = [int(x) for x in f.readline().split()]

graph = Graph(nm[0], nm[1])
for line in f:
if len(line.strip()) > 0:
vx = [int(x) for x in line.split()]
graph.addEdge(vx[0], vx[1])

return graph

def get_components(graph):
"""
Mark graph vertices by components they belong to

:param graph: Graph object to analyze
:return: number of graph components
"""
comp = 0
for i in range(graph.vertices):
if graph.component[i] == -1:
comp += 1
graph.bfs(i, comp)

return comp

def print_components(file_out, graph, ncomp):
"""
Print output to the file specified

:param file_out: path to the output file
:param graph: Graph object to print
:param ncomp: numer of graph components
"""
with open(file_out, 'w') as f:
f.write(str(ncomp))
f.write('\n')
for vc in graph.component:
f.write(str(vc))
f.write(' ')

#Execute
graph = create_graph('components.in')
ncomp = get_components(graph)
print_components('components.out', graph, ncomp)
46 changes: 46 additions & 0 deletions graphs/floyd_warshall.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
__author__ = 'Антон Брагин'

def parse_input(file):
with open(file) as f:
n, m = [int(x) for x in f.readline().split()]

#Create distances matrix. None is placeholder that will be replaces with +Inf
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Можно был не считать total_weights, а просто взять его очень большим: 10**18 например

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

и написать тогда

d = [[10**18 * (i != j) for j in range(n)] for i in range(n)]

d = [[None for j in range(n)] for i in range(n)]

#Vertex always connected to itself with zero distance
for i in range(n):
d[i][i] = 0

total_weight = 0
#Read edges data
for i in range(m):
u, v, w = [int(x) for x in f.readline().split()]
d[u - 1][v - 1] = w
total_weight += w

#Replace None with the value that is greater then any path in graph without negative edges
for i in range(n):
for j in range(n):
if d[i][j] is None:
d[i][j] = total_weight + 1

return n, d

def floyd_warshall(n, d):
for k in range(n):
for i in range(n):
for j in range(n):
if d[i][k] + d[k][j] < d[i][j]:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

min

d[i][j] = d[i][k] + d[k][j]

return d

#Execute
n, d = parse_input('pathsg.in')
d = floyd_warshall(n, d)

#Write output
with open('pathsg.out', 'w') as fout:
for row in d:
fout.write(' '.join(str(x) for x in row))
fout.write('\n')
Loading