Skip to content
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

Add nn.models.GRetriever #9480

Merged
merged 44 commits into from
Sep 10, 2024
Merged

Add nn.models.GRetriever #9480

merged 44 commits into from
Sep 10, 2024

Conversation

puririshi98
Copy link
Contributor

@puririshi98 puririshi98 commented Jul 2, 2024

@puririshi98 puririshi98 self-assigned this Jul 2, 2024
Copy link

codecov bot commented Jul 2, 2024

Codecov Report

Attention: Patch coverage is 14.28571% with 120 lines in your changes missing coverage. Please review.

Project coverage is 87.39%. Comparing base (fbafbc4) to head (dd651be).
Report is 4 commits behind head on master.

Files Patch % Lines
torch_geometric/nn/models/g_retriever.py 17.56% 61 Missing ⚠️
torch_geometric/nn/nlp/llm.py 9.23% 59 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9480      +/-   ##
==========================================
+ Coverage   86.93%   87.39%   +0.46%     
==========================================
  Files         464      478      +14     
  Lines       30755    31236     +481     
==========================================
+ Hits        26737    27299     +562     
+ Misses       4018     3937      -81     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@puririshi98 puririshi98 changed the title G-retriever model integration GRetriever model integration Jul 2, 2024
@akihironitta akihironitta changed the title GRetriever model integration Add nn.models.GRetriever Jul 7, 2024
@puririshi98
Copy link
Contributor Author

@rusty1s @akihironitta anything else needed to merge the 3 remaining PRs?

torch_geometric/nn/models/g_retriever.py Show resolved Hide resolved
torch_geometric/nn/models/g_retriever.py Outdated Show resolved Hide resolved
torch_geometric/nn/models/g_retriever.py Outdated Show resolved Hide resolved
torch_geometric/nn/models/g_retriever.py Show resolved Hide resolved
torch_geometric/nn/nlp/llm.py Outdated Show resolved Hide resolved
@rusty1s
Copy link
Member

rusty1s commented Sep 10, 2024

Very clean now, thanks for the updates :)

@rusty1s rusty1s merged commit 6d9e850 into master Sep 10, 2024
16 checks passed
@rusty1s rusty1s deleted the add-g-retriever branch September 10, 2024 05:33
rusty1s added a commit that referenced this pull request Sep 13, 2024
1. #9462
2. #9480
3. **->** #9481
4. #9167

---

Breaking down PR
#9167 further

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Akihiro Nitta <[email protected]>
Co-authored-by: rusty1s <[email protected]>
rusty1s added a commit that referenced this pull request Sep 13, 2024
1. #9462
2. #9480
3. #9481
4. **->** #9167

---

repro:
Latest NVIDIA PyG container
+
`git config --global credential.helper store; huggingface-cli login; cd
/opt/pyg; pip uninstall -y torch-geometric; rm -rf pytorch_geometric;
git clone -b gnn-llm-model-integration
https://github.com/pyg-team/pytorch_geometric.git; cd
/opt/pyg/pytorch_geometric; pip install .; pip install peft datasets
transformers pcst_fast sentencepiece; python3
examples/llm_plus_gnn/g_retriever.py`

old PR: #9154

note: pure cpu is 220x slower than pure GPU using a single Grace Hopper
(for llama-7b)

info:
tried gemma, performs worse in all train/val/test metrics. most likely
needs some tuning, will leave this as future work as part of the
community sprint to try many LLM and GNN combos and tune them. Therefore
keeping the default llama2

the new gemma-v2 is also much worse than llama2

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Akihiro Nitta <[email protected]>
Co-authored-by: rusty1s <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants