pytorch_geometric icon indicating copy to clipboard operation
pytorch_geometric copied to clipboard

Add `nn.models.GRetriever`

Open puririshi98 opened this issue 1 year ago • 4 comments

  1. https://github.com/pyg-team/pytorch_geometric/pull/9462
  2. -> https://github.com/pyg-team/pytorch_geometric/pull/9480
  3. https://github.com/pyg-team/pytorch_geometric/pull/9481
  4. https://github.com/pyg-team/pytorch_geometric/pull/9167

breaking https://github.com/pyg-team/pytorch_geometric/pull/9167/ down further, focusing on G-retriever model this time

puririshi98 avatar Jul 02 '24 20:07 puririshi98

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 :warning:
torch_geometric/nn/nlp/llm.py 9.23% 59 Missing :warning:
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     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jul 02 '24 21:07 codecov[bot]

I wonder from next time if it would make sense to start adding models to torch_geometric.contrib.nn first and then promote them to torch_geometric.nn when we're confident about the code around them.

akihironitta avatar Jul 26 '24 15:07 akihironitta

I wonder from next time if it would make sense to start adding models to torch_geometric.contrib.nn first and then promote them to torch_geometric.nn when we're confident about the code around them.

from NVIDIAs pov we are already including this in our containers each release so I'd rather save the effort of moving these too pyg.contrib and wait until all of your needs are met before merging these prs. I will address these reviews as soon as i can, and will gladly address any follow up reviews

puririshi98 avatar Jul 29 '24 21:07 puririshi98

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

puririshi98 avatar Aug 02 '24 20:08 puririshi98

Very clean now, thanks for the updates :)

rusty1s avatar Sep 10 '24 04:09 rusty1s