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 remote openai backend to LLM #10078

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Kh4L
Copy link
Contributor

@Kh4L Kh4L commented Feb 27, 2025

No description provided.

@Kh4L Kh4L requested a review from EdisonLeeeee as a code owner February 27, 2025 14:40
@Kh4L Kh4L changed the title Add remote openai backend to LLM Add remote openai backend to LLM Feb 27, 2025
Copy link

codecov bot commented Feb 27, 2025

Codecov Report

Attention: Patch coverage is 4.76190% with 60 lines in your changes missing coverage. Please review.

Project coverage is 86.18%. Comparing base (3f2f43d) to head (93319bf).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
torch_geometric/nn/nlp/llm.py 4.76% 60 Missing ⚠️

❌ Your patch check has failed because the patch coverage (4.76%) 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             @@
##           master   #10078      +/-   ##
==========================================
- Coverage   86.74%   86.18%   -0.56%     
==========================================
  Files         493      493              
  Lines       33069    33105      +36     
==========================================
- Hits        28685    28531     -154     
- Misses       4384     4574     +190     

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

@Kh4L Kh4L force-pushed the add_remote_nlp_llm branch from 93319bf to 7728eca Compare February 28, 2025 02:50
Copy link
Contributor

@puririshi98 puririshi98 left a comment

Choose a reason for hiding this comment

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

this looks good at a high level but i would like to see it integrated into the G-retriever example with argparser flags, and see a succesfull run of both huggingface and openai backends to confirm correctness and reasonable results (with minimal difference between backends

@Kh4L Kh4L force-pushed the add_remote_nlp_llm branch from 38f313a to c0ea2e2 Compare March 3, 2025 09:39
Copy link
Contributor

@puririshi98 puririshi98 left a comment

Choose a reason for hiding this comment

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

please also make it clear that when using the openai backend, it only support frozen LLM, while huggingface support LORA and full finetuning as wel.

It would also be cool if you could add support for LORA with the openai backend

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.

2 participants