Skip to content

Commit

Permalink
Feature/improve time ops (#1371)
Browse files Browse the repository at this point in the history
* Add before and after to TimeOps and change at so it only includes events happening at that point in time (this should fix the semantics for the GraphWithDeletions so the partial_windows workaround is no longer needed)

* Revert "Added 'hard deletion' semantics (#1348)"

f72f97c

* fix edge alive check

* at methods in python and fix tests

* enable the warning comment for pr

* fix benchmark action

* fix the documentation for time ops

* update the impl_timeops docstring

* add docstring for before and after on Edges

---------

Co-authored-by: Haaroon Y <[email protected]>
Co-authored-by: Ben Steer <[email protected]>
  • Loading branch information
3 people authored Nov 15, 2023
1 parent 846abd8 commit f9b8799
Show file tree
Hide file tree
Showing 15 changed files with 466 additions and 732 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/benchmark.yml
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ jobs:
auto-push: false
# Show alert with commit comment on detecting possible performance regression
alert-threshold: '200%'
comment-on-alert: true
github-token: ${{ secrets.GITHUB_TOKEN }}
fail-on-alert: false
save-data-file: false

Expand Down
95 changes: 58 additions & 37 deletions python/tests/test_graphdb.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,9 +280,9 @@ def history_test(key, value):

def time_history_test(time, key, value):
if value is None:
assert g.at(time).properties.temporal.get(key) is None
assert g.before(time+1).properties.temporal.get(key) is None
else:
assert g.at(time).properties.temporal.get(key).items() == value
assert g.before(time+1).properties.temporal.get(key).items() == value

time_history_test(2, "prop 6", [(1, False), (2, True)])
time_history_test(1, "static prop", None)
Expand Down Expand Up @@ -325,7 +325,7 @@ def property_test(key, value):
"prop 5": "world",
"prop 6": True,
}
assert g.at(2).properties.as_dict() == {
assert g.before(3).properties.as_dict() == {
"prop 1": 1,
"prop 2": "hi",
"prop 3": True,
Expand All @@ -341,7 +341,7 @@ def property_test(key, value):
"prop 6": [(1, False), (2, True)],
}

assert g.at(2).properties.temporal.histories() == {
assert g.before(3).properties.temporal.histories() == {
"prop 4": [(1, 11)],
"prop 5": [(1, "world")],
"prop 6": [(1, False), (2, True)],
Expand All @@ -356,14 +356,14 @@ def property_test(key, value):
expected_names_no_static = sorted(["prop 4", "prop 5", "prop 6"])
assert sorted(g.properties.temporal.keys()) == expected_names_no_static

assert sorted(g.at(1).properties.temporal.keys()) == expected_names_no_static
assert sorted(g.before(2).properties.temporal.keys()) == expected_names_no_static

# testing has_property
assert "prop 4" in g.properties
assert "prop 7" not in g.properties
assert "prop 7" not in g.at(1).properties
assert "prop 7" not in g.before(2).properties
assert "prop 1" in g.properties
assert "prop 2" in g.at(1).properties
assert "prop 2" in g.before(2).properties
assert "static prop" not in g.properties.constant


Expand Down Expand Up @@ -415,7 +415,7 @@ def time_history_test(time, key, value):
time_history_test(1, "static prop", None)

def time_static_property_test(time, key, value):
gg = g.at(time)
gg = g.before(time+1)
if value is None:
assert gg.vertex(1).properties.constant.get(key) is None
assert gg.vertices.properties.constant.get(key) is None
Expand All @@ -442,7 +442,7 @@ def static_property_test(key, value):

# testing property
def time_property_test(time, key, value):
gg = g.at(time)
gg = g.before(time+1)
if value is None:
assert gg.vertex(1).properties.get(key) is None
assert gg.vertices.properties.get(key) is None
Expand Down Expand Up @@ -523,21 +523,21 @@ def no_static_property_test(key, value):
"prop 4": [[True]],
}

assert g.at(2).vertex(1).properties == {
assert g.before(3).vertex(1).properties == {
"prop 1": 2,
"prop 4": False,
"prop 2": 0.6,
"static prop": 123,
"prop 3": "hi",
}
assert g.at(2).vertices.properties == {
assert g.before(3).vertices.properties == {
"prop 1": [2],
"prop 4": [False],
"prop 2": [0.6],
"static prop": [123],
"prop 3": ["hi"],
}
assert g.at(2).vertices.out_neighbours.properties == {
assert g.before(3).vertices.out_neighbours.properties == {
"prop 1": [[2]],
"prop 4": [[False]],
"prop 2": [[0.6]],
Expand Down Expand Up @@ -567,17 +567,16 @@ def no_static_property_test(key, value):

assert g.at(2).vertex(1).properties.temporal == {
"prop 2": [(2, 0.6)],
"prop 4": [(1, True), (2, False)],
"prop 1": [(1, 1), (2, 2)],
"prop 3": [(1, "hi")],
"prop 4": [(2, False)],
"prop 1": [(2, 2)],
}
assert g.at(2).vertices.properties.temporal == {
assert g.before(3).vertices.properties.temporal == {
"prop 2": [[(2, 0.6)]],
"prop 4": [[(1, True), (2, False)]],
"prop 1": [[(1, 1), (2, 2)]],
"prop 3": [[(1, "hi")]],
}
assert g.at(2).vertices.out_neighbours.properties.temporal == {
assert g.before(3).vertices.out_neighbours.properties.temporal == {
"prop 2": [[[(2, 0.6)]]],
"prop 4": [[[(1, True), (2, False)]]],
"prop 1": [[[(1, 1), (2, 2)]]],
Expand Down Expand Up @@ -679,7 +678,7 @@ def test_edge_properties():
assert g.at(1).edge(1, 2).properties.temporal.get("static prop") is None

assert g.at(1).edge(1, 2).properties.constant.get("static prop") == 123
assert g.at(100).edge(1, 2).properties.constant.get("static prop") == 123
assert g.before(101).edge(1, 2).properties.constant.get("static prop") == 123
assert g.edge(1, 2).properties.constant.get("static prop") == 123
assert g.edge(1, 2).properties.constant.get("prop 4") is None

Expand Down Expand Up @@ -707,7 +706,7 @@ def test_edge_properties():
"prop 4": True,
}

assert g.at(2).edge(1, 2).properties == {
assert g.before(3).edge(1, 2).properties == {
"prop 1": 2,
"prop 4": False,
"prop 2": 0.6,
Expand All @@ -725,9 +724,14 @@ def test_edge_properties():

assert g.at(2).edge(1, 2).properties.temporal == {
"prop 2": [(2, 0.6)],
"prop 4": [(1, True), (2, False)],
"prop 1": [(1, 1), (2, 2)],
"prop 3": [(1, "hi")],
"prop 4": [(2, False)],
"prop 1": [(2, 2)],
}

assert g.after(2).edge(1, 2).properties.temporal == {
"prop 2": [(3, 0.9)],
"prop 3": [(3, "hello")],
"prop 4": [(3, True)],
}

# testing property names
Expand Down Expand Up @@ -875,13 +879,21 @@ def test_save_load_graph():
def test_graph_at():
g = create_graph()

view = g.at(2)
view = g.at(1)
assert view.vertex(1).degree() == 2
assert view.vertex(2).degree() == 1

view = g.before(3)
assert view.vertex(1).degree() == 3
assert view.vertex(3).degree() == 1

view = g.at(7)
view = g.before(8)
assert view.vertex(3).degree() == 2

view = g.after(6)
assert view.vertex(2).degree() == 1
assert view.vertex(3).degree() == 1


def test_add_node_string():
g = Graph()
Expand Down Expand Up @@ -917,7 +929,7 @@ def test_all_neighbours_window():
g.add_edge(3, 3, 2, {})
g.add_edge(4, 2, 4, {})

view = g.at(2)
view = g.before(3)
v = view.vertex(2)
assert list(v.window(0, 2).in_neighbours.id) == [1]
assert list(v.window(0, 2).out_neighbours.id) == [3]
Expand All @@ -934,7 +946,7 @@ def test_all_degrees_window():
g.add_edge(4, 2, 4, {})
g.add_edge(5, 2, 1, {})

view = g.at(4)
view = g.before(5)
v = view.vertex(2)
assert v.window(0, 4).in_degree() == 3
assert v.window(start=2).in_degree() == 2
Expand All @@ -957,7 +969,7 @@ def test_all_edge_window():
g.add_edge(4, 2, 4, {})
g.add_edge(5, 2, 1, {})

view = g.at(4)
view = g.before(5)
v = view.vertex(2)
assert sorted(v.window(0, 4).in_edges.src.id) == [1, 3, 4]
assert sorted(v.window(end=4).in_edges.src.id) == [1, 3, 4]
Expand Down Expand Up @@ -1012,8 +1024,7 @@ def test_triplet_count():
g.add_edge(0, 2, 3, {})
g.add_edge(0, 3, 1, {})

v = g.at(1)
assert algorithms.triplet_count(v) == 3
assert algorithms.triplet_count(g) == 3


def test_global_clustering_coeffficient():
Expand All @@ -1026,8 +1037,8 @@ def test_global_clustering_coeffficient():
g.add_edge(0, 4, 1, {})
g.add_edge(0, 5, 2, {})

v = g.at(1)
assert algorithms.global_clustering_coefficient(v) == 0.5454545454545454
assert algorithms.global_clustering_coefficient(g) == 0.5454545454545454
assert algorithms.global_clustering_coefficient(g.at(0)) == 0.5454545454545454


def test_edge_time_apis():
Expand Down Expand Up @@ -1082,8 +1093,13 @@ def test_edge_earliest_latest_time():

assert list(g.vertex(1).edges.earliest_time) == [0, 0]
assert list(g.vertex(1).edges.latest_time) == [2, 2]
assert list(g.vertex(1).at(1).edges.earliest_time) == [0, 0]
assert list(g.vertex(1).at(1).edges.earliest_time) == [1, 1]
assert list(g.vertex(1).before(1).edges.earliest_time) == [0, 0]
assert list(g.vertex(1).after(1).edges.earliest_time) == [2, 2]
assert list(g.vertex(1).at(1).edges.latest_time) == [1, 1]
assert list(g.vertex(1).before(1).edges.latest_time) == [0, 0]
assert list(g.vertex(1).after(1).edges.latest_time) == [2, 2]



def test_vertex_earliest_time():
Expand All @@ -1093,9 +1109,14 @@ def test_vertex_earliest_time():
g.add_vertex(2, 1, {})

view = g.at(1)
assert view.vertex(1).earliest_time == 0
assert view.vertex(1).earliest_time == 1
assert view.vertex(1).latest_time == 1
view = g.at(3)

view = g.after(0)
assert view.vertex(1).earliest_time == 1
assert view.vertex(1).latest_time == 2

view = g.before(3)
assert view.vertex(1).earliest_time == 0
assert view.vertex(1).latest_time == 2

Expand Down Expand Up @@ -1227,8 +1248,8 @@ def test_lotr_edge_history():
31445,
32656,
]
assert g.at(1000).edge("Frodo", "Gandalf").history() == [329, 555, 861]
assert g.edge("Frodo", "Gandalf").at(1000).history() == [329, 555, 861]
assert g.before(1000).edge("Frodo", "Gandalf").history() == [329, 555, 861]
assert g.edge("Frodo", "Gandalf").before(1000).history() == [329, 555, 861]
assert g.window(100, 1000).edge("Frodo", "Gandalf").history() == [329, 555, 861]
assert g.edge("Frodo", "Gandalf").window(100, 1000).history() == [329, 555, 861]

Expand Down Expand Up @@ -1316,7 +1337,7 @@ def test_window_size():
g.add_vertex(1, 1)
g.add_vertex(4, 4)

assert g.window_size() == 4
assert g.window_size == 4


def test_time_index():
Expand Down
4 changes: 1 addition & 3 deletions raphtory/src/algorithms/metrics/clustering_coefficient.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,7 @@ mod cc_test {
graph.add_edge(0, src, dst, NO_PROPS, None).unwrap();
}

let graph_at = graph.at(1);

let results = clustering_coefficient(&graph_at);
let results = clustering_coefficient(&graph);
assert_eq!(results, 0.3);
}
}
2 changes: 1 addition & 1 deletion raphtory/src/algorithms/motifs/triplet_count.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ mod triplet_test {
graph.add_edge(0, src, dst, NO_PROPS, None).unwrap();
}
let exp_triplet_count = 20;
let results = triplet_count(&graph.at(1), None);
let results = triplet_count(&graph, None);

assert_eq!(results, exp_triplet_count);
}
Expand Down
27 changes: 22 additions & 5 deletions raphtory/src/db/api/view/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,31 @@ pub trait TimeOps {
/// Create a view including all events between `start` (inclusive) and `end` (exclusive)
fn window<T: IntoTime>(&self, start: T, end: T) -> Self::WindowedViewType;

/// Create a view including all events until `end` (inclusive)
fn at<T: IntoTime>(&self, end: T) -> Self::WindowedViewType {
/// Create a view that only includes events at `time`
fn at<T: IntoTime>(&self, time: T) -> Self::WindowedViewType {
let start = time.into_time();
self.window(start, start.saturating_add(1))
}

/// Create a view that only includes events after `start` (exclusive)
fn after<T: IntoTime>(&self, start: T) -> Self::WindowedViewType {
let start = start.into_time().saturating_add(1);
let end = self.end().unwrap_or(start.saturating_add(1));
if end < start {
self.window(start, start)
} else {
self.window(start, end)
}
}

/// Create a view that only includes events before `end` (exclusive)
fn before<T: IntoTime>(&self, end: T) -> Self::WindowedViewType {
let end = end.into_time();
let start = self.start().unwrap_or(end);
if start > end {
self.window(end, end.saturating_add(1))
if end < start {
self.window(end, end)
} else {
self.window(start, end.saturating_add(1))
self.window(start, end)
}
}

Expand Down
Loading

1 comment on commit f9b8799

@Haaroon
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Rust Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: f9b8799 Previous: 846abd8 Ratio
lotr_graph/has_edge_existing 79 ns/iter (± 2) 36 ns/iter (± 0) 2.19
lotr_graph/has_edge_nonexisting 78 ns/iter (± 2) 35 ns/iter (± 1) 2.23
lotr_graph/has_vertex_existing 25 ns/iter (± 0) 10 ns/iter (± 0) 2.50
lotr_graph/has_vertex_nonexisting 25 ns/iter (± 0) 8 ns/iter (± 0) 3.13
lotr_graph/max_id 5193 ns/iter (± 6) 1819 ns/iter (± 56) 2.85
lotr_graph_window_100/has_edge_existing 104 ns/iter (± 6) 43 ns/iter (± 6) 2.42
lotr_graph_window_100/has_edge_nonexisting 77 ns/iter (± 2) 36 ns/iter (± 1) 2.14
lotr_graph_window_100/has_vertex_nonexisting 26 ns/iter (± 0) 8 ns/iter (± 0) 3.25
lotr_graph_window_10/has_edge_existing 104 ns/iter (± 6) 43 ns/iter (± 4) 2.42
lotr_graph_window_10/has_edge_nonexisting 77 ns/iter (± 5) 34 ns/iter (± 1) 2.26
lotr_graph_window_10/has_vertex_nonexisting 26 ns/iter (± 0) 8 ns/iter (± 0) 3.25

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.