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

Graph.delete_vertex/2 does not remove references in in_edges #17

Closed
zyqxd opened this issue Apr 17, 2018 · 2 comments
Closed

Graph.delete_vertex/2 does not remove references in in_edges #17

zyqxd opened this issue Apr 17, 2018 · 2 comments

Comments

@zyqxd
Copy link

zyqxd commented Apr 17, 2018

Given

graph =
  Graph.new
  |> Graph.add_vertices([1, 2, 4, 6])
  |> Graph.add_edge(1, 2)
  |> Graph.add_edge(2, 4)
  |> Graph.add_edge(4, 6)
  
graph_two = 
  graph
  |> Graph.add_vertices([3, 5, 7])
  |> Graph.add_edge(1, 3)
  |> Graph.add_edge(3, 4)
  |> Graph.add_edge(3, 5)
  |> Graph.add_edge(5, 6)
  |> Graph.add_edge(5, 7)

Test

assert graph == Graph.delete_vertices(graph_two, [3, 5, 7])
# The result is false

Inspect

Map.from_struct graph_two
%{
  edges: %{
    {539485162, 2577631668} => %{nil: 1},
    {1379807339, 3239926044} => %{nil: 1},
    {2577631668, 1379807339} => %{nil: 1}
  },
  in_edges: %{
    1379807339 => #MapSet<[1186525603, 2577631668]>,  #1186525603 dangling reference
    2577631668 => #MapSet<[539485162]>,
    3239926044 => #MapSet<[676967202, 1379807339]> #676967202 dangling reference
  },
  out_edges: %{
    539485162 => #MapSet<[2577631668]>,
    1379807339 => #MapSet<[3239926044]>,
    2577631668 => #MapSet<[1379807339]>
  },
  type: :directed,
  vertex_labels: %{
    539485162 => [],
    1379807339 => [],
    2577631668 => [],
    3239926044 => []
  },
  vertices: %{539485162 => 1, 1379807339 => 4, 2577631668 => 2, 3239926044 => 6}
}

These dangling references also raises nil enumeration errors when performing future operations.

Offending line

Graph.delete_vertex/2 is missing comprehension for in edges

Fix is to add

ie = for {id, ns} <- ie, do: {id, MapSet.delete(ns, v_id)}, into: %{}
@aquarhead
Copy link
Contributor

Just encountered this as well... @itsdeezy can you make a PR? Confirmed your fix works for me :)

@bitwalker
Copy link
Owner

Fixed in master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants