Repository navigation
Rolling and expanding window alignment based on the user's time interval input - #2277
Merged
Merged
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'GraphQL Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.
| Benchmark suite | Current: 7fafdff | Previous: f14de7d | Ratio |
|---|---|---|---|
addNode |
19 req/s |
1460 req/s |
76.84 |
This comment was automatically generated by workflow using github-action-benchmark.
…e like expanding() and rolling() windows but are aligned at the start. They get aligned at the smallest unit of time passed as input.
arienandalibi
force-pushed
the
fix/align-windows
branch
from
September 16, 2025 05:08
5eb0939 to
3b381e5
Compare
…() functions. Added align_start flag in those functions as well. Added python tests for alignment of rolling and expanding windows, both for Python and GraphQL. Changed logic so alignment also happens on step if it is provided.
arienandalibi
force-pushed
the
fix/align-windows
branch
from
September 18, 2025 08:56
9416bb2 to
9856650
Compare
miratepuffin
approved these changes
Sep 18, 2025
…e mismatched discrete/temporal intervals for window and step. Tests updated to reflect that.
…nt types, such as node, nodes, edge, edges, path_from_node, path_from_graph, and for mismatched window and step types.
arienandalibi
force-pushed
the
fix/align-windows
branch
from
September 22, 2025 03:24
3a8267a to
8c3c408
Compare
… a step is not passed, then the step defaults to the window). Adjusted tests accordingly. Update window() documentation to not say start and end are optional
arienandalibi
force-pushed
the
fix/align-windows
branch
from
September 24, 2025 05:36
0bb0f0a to
9cafdd2
Compare
ljeub-pometry
requested changes
Sep 24, 2025
ljeub-pometry
left a comment
ljeub-pometry
left a comment
Collaborator
There was a problem hiding this comment.
- Some suggestions for the rust api
- tests should check the end of the rolling as well and make sure it is sensible (they look only at the first 3 windows in a lot of cases right now)
…indow can lead to entries being outside of all windows (before start and/or after end). Updated rolling/expanding tests to verify the last window as well (test boundaries). Added some tests for different step/window combinations.
arienandalibi
force-pushed
the
fix/align-windows
branch
from
September 25, 2025 06:37
5df7e26 to
2f92fc9
Compare
…ling_aligned()/expanding_aligned() now take an AlignmentUnit parameter for custom alignment.
…and expanding() are checked for all types.
…tional string alignment_unit parameter for custom alignment. If no alignment_unit is passed, aligns on the smallest unit like before. "unaligned" allows for no alignment.
…ptional enum alignment_unit parameter for custom alignment, like in Python. If no alignment_unit is passed, aligns on the smallest unit like before. "unaligned" allows for no alignment.
# Conflicts: # pometry-storage-private # raphtory/src/algorithms/motifs/temporal_rich_club_coefficient.rs # raphtory/src/db/api/view/time.rs # raphtory/src/db/graph/graph.rs
… window, step, and alignment.
arienandalibi
force-pushed
the
fix/align-windows
branch
from
October 6, 2025 21:13
dc48242 to
742e4f8
Compare
…d (it previously did). If we do `g.layer("x").rolling("1 month")`, the first window will start aligned with the graph's first event, not the first event on layer "x" (even if the first window will be empty)
…f continuous addition. This fixes the previous bug caused by: Jan 31st + "1 month" = Feb 28th; if this happens on the window's end, all following windows will be on the 28th (instead of 29th, 30th, or 31st). This also used to happen for windows always ending on the 30th even if they should end on the 31st. Added and fixed tests for it as well.
miratepuffin
marked this pull request as ready for review
October 15, 2025 14:52
miratepuffin
approved these changes
Oct 15, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Added rolling_aligned() and expanding_aligned() functions which behave like rolling() and expanding() windows except that the start of each window is aligned with the smallest unit of time passed by the user. For example, if the interval is "1 month and 1 day", the first window will begin at the start of the day of the first time event. The non-aligned versions of these functions begin the window at the first time event. When used in Python, aligning is now the default and has to be disabled using align_start = False if desired.
Why are the changes needed?
It's more intuitive for windows to be aligned cleanly. If someone wants a rolling window over "1 year", they likely want it to start at the beginning of the calendar year and end at the end of the year.
Does this PR introduce any user-facing change? If yes is this documented?
The functions expanding() and rolling() now have a new option parameter called align_start, which allows users to disable the alignment (use the same functionality as before).