Skip to content

Time semantics overhaul#1969

Merged
ljeub-pometry merged 197 commits intomasterfrom
feature/IsValidFilter
Jun 6, 2025
Merged

Time semantics overhaul#1969
ljeub-pometry merged 197 commits intomasterfrom
feature/IsValidFilter

Conversation

@ljeub-pometry
Copy link
Collaborator

@ljeub-pometry ljeub-pometry commented Mar 6, 2025

What changes were proposed in this pull request?

  • refactor of the time semantics to simplify history filtering
  • filtering layers/edges now filters nodes as well if all the edge updates that added them are filtered out and the node is not added explicitly via add_node.
    • as a result, subgraph filters nodes that don't have edges in the subgraph and were not explicitly added via add_node.
  • changes latest_time semantics for the PersistentGraph to return the time of the last update for the node/edge/graph in the current view or the start of the window if there are no updates.
  • The earliest_time and latest_time within a filtered Event Graph will now reflect the updates within the graph view instead of just window bounds.
  • adds a ValidGraph filter that only keeps edges that are currently valid without removing their history
  • is_valid and is_active are no longer the same for the persistent graph
    • active means there is an update during the period (addition or deletion)
    • valid means that the edges most recent update is an addition (persistent semantics).
    • deleted means that the edges most recent update is a deletion.
  • the event graph preserves deletions if created from a persistent graph
    • an edge is included in a window if it is active in the window (has an addition or deletion event)
    • an edge is valid if it has an addition event in the current view
    • an edge is deleted if it has an addition event in the current view
  • The default layer only exists if it has updates on it.

Why are the changes needed?

Fixes #1050, fixes #1787, fixes #1191
Fixes #1779

Does this PR introduce any user-facing change? If yes is this documented?

How was this patch tested?

  • adds tests for the new semantics
  • more proptests to check that materializing views gives consistent results

Are there any further changes required?

@ljeub-pometry ljeub-pometry force-pushed the feature/IsValidFilter branch from 7f2d03d to dfbb7a8 Compare March 10, 2025 12:58
@ljeub-pometry ljeub-pometry force-pushed the feature/IsValidFilter branch from 14b8b49 to bab1997 Compare March 24, 2025 13:11
Copy link
Contributor

@github-actions github-actions bot left a 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: 156f747 Previous: 48a104d Ratio
lotr_graph_window_10/iterate edges 59020 ns/iter (± 131) 28682 ns/iter (± 97) 2.06
lotr_graph_window_50_layered/iterate edges 104087 ns/iter (± 556) 49863 ns/iter (± 251) 2.09
lotr_graph_persistent_window_50_layered/num_edges_temporal 259529 ns/iter (± 2394) 121480 ns/iter (± 1820) 2.14
lotr_graph_persistent_window_50_layered/has_node_existing 121 ns/iter (± 75) 26 ns/iter (± 1) 4.65
lotr_graph_persistent_window_50_layered/iterate nodes 28892 ns/iter (± 53) 8586 ns/iter (± 60) 3.37

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

@CLAassistant
Copy link

CLAassistant commented Mar 24, 2025

CLA assistant check
All committers have signed the CLA.

Copy link
Collaborator

@fabianmurariu fabianmurariu left a comment

Choose a reason for hiding this comment

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

LGTM

#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Default, Serialize, Deserialize)]
pub struct ELID {
pub edge: EID,
layer_and_deletion: usize,
Copy link
Collaborator

Choose a reason for hiding this comment

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

@ljeub-pometry ljeub-pometry merged commit e98833a into master Jun 6, 2025
29 checks passed
@ljeub-pometry ljeub-pometry deleted the feature/IsValidFilter branch June 6, 2025 12:24
@fabianmurariu fabianmurariu restored the feature/IsValidFilter branch June 9, 2025 12:09
@miratepuffin miratepuffin deleted the feature/IsValidFilter branch July 4, 2025 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants