-
Notifications
You must be signed in to change notification settings - Fork 56
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
Feature/latest and active #1786
Conversation
There was a problem hiding this comment.
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: f5885c2 | Previous: b992030 | Ratio |
---|---|---|---|
lotr_graph_subgraph_10pc/max_degree |
48261 ns/iter (± 6862 ) |
22968 ns/iter (± 1168 ) |
2.10 |
lotr_graph_subgraph_10pc_materialise/materialize |
825371 ns/iter (± 87894 ) |
225687 ns/iter (± 4926 ) |
3.66 |
lotr_graph_window_50_layered/max_id |
49804 ns/iter (± 3959 ) |
22137 ns/iter (± 1414 ) |
2.25 |
This comment was automatically generated by workflow using github-action-benchmark.
…work with windowing the graph
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -111,6 +111,14 @@ macro_rules! impl_timeops { | |||
self.$field.at(time) | |||
} | |||
|
|||
#[doc = concat!(r" Create a view of the ", $name, r" including all events at the latest time.")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
latest time in the Graph
What changes were proposed in this pull request?
latest()
function on all classes which equates tox.at(graph.latest_time)
is_active
on edge, edges, and node as a short hand to see if the entitiy has any updates within the window - this is not added only nodes as they are filtered through windows so it would basically always return true.TODO:
Why are the changes needed?
Does this PR introduce any user-facing change? If yes is this documented?
Yes and yes
How was this patch tested?
New tests
Issues
Created #1787
Are there any further changes required?
This has opened a couple of questions on if other filters should apply a window, and how should the DSL filtering be read.