-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Add Annotation to Stored Triples #10410
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
base: latest-txt2kg
Are you sure you want to change the base?
Add Annotation to Stored Triples #10410
Conversation
for more information, see https://pre-commit.ci
…u/pytorch_geometric into add-annotation-to-triples
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 overall, left a small comment
@@ -322,8 +322,15 @@ def index_kg(args, context_docs): | |||
checkpoint_path = os.path.join(args.dataset, "checkpoint_kg.pt") | |||
if os.path.exists(checkpoint_path): | |||
print("Restoring KG from checkpoint...") | |||
saved_relevant_triples = torch.load(checkpoint_path, | |||
weights_only=False) |
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.
why did you remove weights_only=False
?
i think you should leave that arg
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.
I thought weights_only
was only useful if we want to extract model weights and biases, aka, tensors. Otherwise, isn't the argument not really useful in this situation? I can add it back if that's incorrect
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.
AFAIK thats the case when you set True
, here we want to load items that are not Tensors (W&B), so we need to set it to False
, otherwise pytorch might complaint
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.
Ah okay. I thought it was okay to not use it since this Data object is never storing weights and biases. I'll add that back in.
for more information, see https://pre-commit.ci
…u/pytorch_geometric into add-annotation-to-triples
for more information, see https://pre-commit.ci
examples/llm/txt2kg_rag.py
Outdated
@@ -409,12 +418,17 @@ def make_dataset(args): | |||
triples = [] | |||
raw_triples_path = os.path.join(args.dataset, "raw_triples.pt") | |||
if os.path.exists(raw_triples_path): | |||
triples = torch.load(raw_triples_path, weights_only=False) | |||
saved_data = torch.load(raw_triples_path) |
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.
plz save/load w format {llm-name}{datetime}raw_triples.pt
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.
changed format
also plz make CI green, check the failures and fix |
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (50.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## latest-txt2kg #10410 +/- ##
================================================
Coverage ? 85.08%
================================================
Files ? 507
Lines ? 35456
Branches ? 0
================================================
Hits ? 30166
Misses ? 5290
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…u/pytorch_geometric into add-annotation-to-triples
for more information, see https://pre-commit.ci
…u/pytorch_geometric into add-annotation-to-triples
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
It looks like CI is failing here but I'm not sure why. The files I touched shouldn't affect the tests, right? |
yeah i think we're safe skipping it |
Closes https://github.com/rapidsai/graph_dl/issues/820
This PR adds support for storing the model used for generating triplets in the
txt2kg_rag
workflow.