Skip to content

Enable Constructing DGraph with TimeDelta as String - #75

Merged
Jacob-Chmura merged 8 commits into
mainfrom
dev/time_delta_as_string
Jul 2, 2025
Merged

Enable Constructing DGraph with TimeDelta as String#75
Jacob-Chmura merged 8 commits into
mainfrom
dev/time_delta_as_string

Conversation

@Jacob-Chmura

@Jacob-Chmura Jacob-Chmura commented Jun 27, 2025

Copy link
Copy Markdown
Member

Purpose

The purpose of this PR is to enable users to construct a DGraph with the time_delta as a string:

DGraph('tgbl-wiki', time_delta='s') # instead of `DGraph('tgbl-wiki', time_delta=TimeDeltaDG('s'))

which seems like a nice usability feature. User never has to know about TimeDeltaDG anymore.

Key Changes

  • Change DGraph constructor to accept string option
  • Updated examples and tests to reflect this logic

Relevant Prs

Close #72

@Jacob-Chmura Jacob-Chmura self-assigned this Jun 27, 2025
@shenyangHuang

shenyangHuang commented Jul 1, 2025

Copy link
Copy Markdown
Collaborator

Power users should still use TimeDelta, say they want 6 months as a time granularity. Agree that this is more intuitive for normal use.

@shenyangHuang shenyangHuang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

PR is approved, let's discuss how to iterate over fixed number of edges even for non-r time granularities

train_dg = DGraph(args.dataset, time_delta=TimeDeltaDG('r'), split='train')
val_dg = DGraph(args.dataset, time_delta=TimeDeltaDG('r'), split='val')
test_dg = DGraph(args.dataset, time_delta=TimeDeltaDG('r'), split='test')
train_dg = DGraph(args.dataset, split='train')

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nice

Comment thread test/test_dataloader.py
edge_timestamps = torch.LongTensor([0, 1, 2, 3, 4, 5, 6, 7, 8, 9])
data = DGData.from_raw(edge_timestamps, edge_index)
dg = DGraph(data, time_delta=TimeDeltaDG('s'))
dg = DGraph(data, time_delta='s')

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can we still iterate by 200 edges when we have a 's' granularity?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this PR is fine, but this point we can discuss at meeting

@codecov

codecov Bot commented Jul 2, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@Jacob-Chmura
Jacob-Chmura merged commit 37f63ba into main Jul 2, 2025
5 checks passed
@Jacob-Chmura
Jacob-Chmura deleted the dev/time_delta_as_string branch July 2, 2025 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Let DGraph constructor take TimeDelta as a string

2 participants