Skip to content
This repository was archived by the owner on Apr 10, 2020. It is now read-only.
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
132 changes: 132 additions & 0 deletions __tests__/dijkstra_test.re
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
open Jest;
open Expect;

describe("Breadth First Search", () => {
open Dijkstra;

let infinity = Pervasives.max_int;

test("single node", () => {

let adj_list = [{id: "A", neighbours: []}];

let expected_dist = 0;
let expected_prev = None;

let {dist, prev} = search(adj_list, [], "A");
let dist = Hashtbl.find(dist, "A");
let prev = Hashtbl.find(prev, "A");

expect((dist, prev)) |> toEqual((expected_dist, expected_prev));
});

test("simple unidirectional cycle", () => {

let adj_list = [
{id: "A", neighbours: ["B"]},
{id: "B", neighbours: ["C"]},
{id: "C", neighbours: ["D"]},
{id: "D", neighbours: ["A"]}];

let weight_list = [
{tail: "A", head: "B", weight: 1},
{tail: "B", head: "C", weight: 1},
{tail: "C", head: "D", weight: 1},
{tail: "D", head: "A", weight: 1}]

let expected_dists = (0, 1, 2, 3);
let expected_prevs = (None, Some("A"), Some("B"), Some("C"));

let {dist, prev} = search(adj_list, weight_list, "A");

let dists = (
Hashtbl.find(dist, "A"),
Hashtbl.find(dist, "B"),
Hashtbl.find(dist, "C"),
Hashtbl.find(dist, "D"));

let prevs = (
Hashtbl.find(prev, "A"),
Hashtbl.find(prev, "B"),
Hashtbl.find(prev, "C"),
Hashtbl.find(prev, "D"));

expect((dists, prevs)) |> toEqual((expected_dists, expected_prevs));
});

test("complete biderectional graph", () => {

let adj_list = [
{id: "A", neighbours: ["B", "C", "D"]},
{id: "B", neighbours: ["A", "C", "D"]},
{id: "C", neighbours: ["A", "B", "D"]},
{id: "D", neighbours: ["A", "B", "C"]}];

let weight_list = [
{tail: "A", head: "B", weight: 2},
{tail: "A", head: "C", weight: 5},
{tail: "A", head: "D", weight: 5},
{tail: "B", head: "A", weight: 1},
{tail: "B", head: "C", weight: 1},
{tail: "B", head: "D", weight: 3},
{tail: "C", head: "A", weight: 1},
{tail: "C", head: "B", weight: 1},
{tail: "C", head: "D", weight: 1},
{tail: "D", head: "A", weight: 1},
{tail: "D", head: "B", weight: 1},
{tail: "D", head: "C", weight: 1}]

let expected_dists = (0, 2, 3, 4);
let expected_prevs = (None, Some("A"), Some("B"), Some("C"));

let {dist, prev} = search(adj_list, weight_list, "A");

let dists = (
Hashtbl.find(dist, "A"),
Hashtbl.find(dist, "B"),
Hashtbl.find(dist, "C"),
Hashtbl.find(dist, "D"));

let prevs = (
Hashtbl.find(prev, "A"),
Hashtbl.find(prev, "B"),
Hashtbl.find(prev, "C"),
Hashtbl.find(prev, "D"));

expect((dists, prevs)) |> toEqual((expected_dists, expected_prevs));
});

test("forest", () => {

let adj_list = [
{id: "A", neighbours: ["B"]},
{id: "B", neighbours: ["A"]},
{id: "C", neighbours: ["D"]},
{id: "D", neighbours: ["C"]}];

let weight_list = [
{tail: "A", head: "B", weight: 1},
{tail: "B", head: "A", weight: 1},
{tail: "C", head: "D", weight: 1},
{tail: "D", head: "C", weight: 1}]

let expected_dists = (0, 1, infinity, infinity);
let expected_prevs = (None, Some("A"), None, None);

let {dist, prev} = search(adj_list, weight_list, "A");

let dists = (
Hashtbl.find(dist, "A"),
Hashtbl.find(dist, "B"),
Hashtbl.find(dist, "C"),
Hashtbl.find(dist, "D"));

let prevs = (
Hashtbl.find(prev, "A"),
Hashtbl.find(prev, "B"),
Hashtbl.find(prev, "C"),
Hashtbl.find(prev, "D"));

expect((dists, prevs)) |> toEqual((expected_dists, expected_prevs));
});
});
151 changes: 151 additions & 0 deletions src/graphs/Dijkstra.re
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
type node = {
id: string,
neighbours: list(string),
};

type directedGraph = list(node);

type edge = {
tail: string,
head: string,
weight: int,
};

type resultType = {
dist: Hashtbl.t(string, int),
prev: Hashtbl.t(string, option(string))
};

exception Not_found(string);

let infinity = Pervasives.max_int;

let parseAdjList = adj_list => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should construct the adj_table from the edges instead. We don't need to specify the adj_list at all!

let adj_tbl = Hashtbl.create(List.length(adj_list));
let insert = ({id, neighbours}) => {
Hashtbl.add(adj_tbl, id, neighbours);
};

List.iter(insert, adj_list);
let validateNeighbours = node => {
List.iter(neighbour => {
if (!Hashtbl.mem(adj_tbl, neighbour)) {
raise(Not_found(neighbour));
}
}, node.neighbours);
};

List.iter(validateNeighbours, adj_list);
adj_tbl;
};

let parseWeights = (weight_list, adj_list) => {
let weight_tbl = Hashtbl.create(List.length(adj_list));
let initializeWeights = ({id, neighbours}) => {
let adj_edge_weights = Hashtbl.create(List.length(neighbours));
Hashtbl.add(weight_tbl, id, adj_edge_weights);
};

List.iter(initializeWeights, adj_list);
Copy link
Collaborator

Choose a reason for hiding this comment

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

why creating a nested Hash here? Why not using a tuple as the key?

let insertWeight = ({tail, head, weight}) => {
let adj_edge_weights = Hashtbl.find(weight_tbl, tail);
Hashtbl.add(adj_edge_weights, head, weight);
};

List.iter(insertWeight, weight_list);
weight_tbl;
};

let initializeDist = (adj_list, source_id) => {
let dist = Hashtbl.create(List.length(adj_list));
let initialize = ({id, _}) => {
Hashtbl.add(dist, id, infinity);
};

List.iter(initialize, adj_list);
Hashtbl.replace(dist, source_id, 0);
dist;
};

let initializePrev = adj_list => {
let prev = Hashtbl.create(List.length(adj_list));
let initialize = ({id, _}) => {
Hashtbl.add(prev, id, None);
};

List.iter(initialize, adj_list);
prev;
};

let getEdgeWeight = (weight_tbl, tail, head) => {
let adj_edge_weights = Hashtbl.find(weight_tbl, tail);
Hashtbl.find(adj_edge_weights, head);
};

let add = (a, b) => {
if (a == infinity || b == infinity) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we should use infinity. Pervasive.infinity is for floating points but here we are dealing with int. The distance hash_tbl can map to option(int)

Copy link
Member Author

Choose a reason for hiding this comment

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

I used let infinity = Pervasives.max_int; not let infinity = Pervasives.infinity.
So should I still change this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

debatable but i think it's fine.

infinity;
} else {
a + b;
};
};

let rec traverse = (~queue, ~adj_tbl, ~weight_tbl, ~dist, ~prev) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think queue should be renamed to frontier?

switch (Heap.size(queue)) {
| 0 => ();
| _ => {
let current_id = Heap.extract(queue);
let neighbours = Hashtbl.find(adj_tbl, current_id);

let updateDist = neighbour_id => {
let neighbour_dist = Hashtbl.find(dist, neighbour_id);
let current_dist = Hashtbl.find(dist, current_id);
let edge_weight = getEdgeWeight(weight_tbl, current_id, neighbour_id);
let alt_dist = add(current_dist, edge_weight);

if (alt_dist < neighbour_dist) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe we should refactor this and call it relax?

Copy link
Member Author

Choose a reason for hiding this comment

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

You mean relax instead of alt_dist?

Copy link
Collaborator

Choose a reason for hiding this comment

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

my comment was for the content of the if statement

Hashtbl.replace(dist, neighbour_id, alt_dist);
Hashtbl.replace(prev, neighbour_id, Some(current_id));
Heap.remove(queue, (key, _) => key == neighbour_id) |> ignore;
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we add update function to the Heap?

Copy link
Member Author

Choose a reason for hiding this comment

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

I added an update function to the Heap

Copy link
Collaborator

Choose a reason for hiding this comment

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

I approved your PR

Heap.add(queue, neighbour_id, neighbour_id);
};
};

List.iter(updateDist, neighbours);
traverse(
~queue=queue,
~adj_tbl=adj_tbl,
~weight_tbl=weight_tbl,
~dist=dist,
~prev=prev);
};
};
};

let search = (adj_list, weights, source_id) => {
let prev = initializePrev(adj_list);
let dist = initializeDist(adj_list, source_id);
let weight_tbl = parseWeights(weights, adj_list);
let adj_tbl = parseAdjList(adj_list);

let compare = (a, b) => {
let dist_a = Hashtbl.find(dist, a);
let dist_b = Hashtbl.find(dist, b);
dist_a < dist_b;
};

let priorityQueue = Heap.create(compare);
let enqueue = (key, _) => {
Heap.add(priorityQueue, key, key);
};

Hashtbl.iter(enqueue, dist);
traverse(
~queue=priorityQueue,
~adj_tbl=adj_tbl,
~weight_tbl=weight_tbl,
~dist=dist,
~prev=prev);

{dist: dist, prev: prev};
};
19 changes: 19 additions & 0 deletions src/graphs/Dijkstra.rei
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
type node = {
id: string,
neighbours: list(string),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does a node need a set of neighbors? We can calculate them based on the edges.

};

type edge = {
tail: string,
head: string,
weight: int,
};

type directedGraph = list(node);
Copy link
Collaborator

Choose a reason for hiding this comment

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

why are you passing in the edges outside of the directedGraph?

Copy link
Collaborator

Choose a reason for hiding this comment

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

why don't we only specify the edges and construct the nodes inside the module?

Copy link
Member Author

@selfeki selfeki Sep 5, 2018

Choose a reason for hiding this comment

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

What if a node does not have any incident edges? I think we still need a list of nodes but without the neighbours field.


type resultType = {
dist: Hashtbl.t(string, int),
prev: Hashtbl.t(string, option(string))
};

let search: (directedGraph, list(edge), string) => resultType;