-
Notifications
You must be signed in to change notification settings - Fork 442
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
feat(learning): feature_store & graph_store V1 #4237
base: main
Are you sure you want to change the base?
Conversation
@Yi-Eaaa Could you make sure that the PR-Check has passed? |
r"""To be implemented by :class:`GSFeatureStore`.""" | ||
raise NotImplementedError | ||
|
||
def _get_tensor_size(self, attr: TensorAttr) -> Optional[Tuple[int, ...]]: |
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.
how do these methods _get_tensor_size
get_all_tensor_attrs
_build_features
work?
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.
TensorAttr stores various attributes that uniquely represent the vertex/edge feature.
_get_tensor_size gets the length of the vertex feature corresponding to TensorAttr.
get_all_tensor_attrs retrieves all TensorAttrs that exist in the FeatureStore.
_build_features is what I referred to in dist_data earlier, it doesn't work here, I forgot to delete it.
r"""To be implemented by :class:`GSFeatureStore`.""" | ||
raise NotImplementedError | ||
|
||
def get_all_edge_attrs(self) -> List[EdgeAttr]: |
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.
how does get_all_edge_attrs
work?
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.
get_all_edge_attrs returns the EdgeAttr for all types of edges of the graph. An EdgeAttr can uniquely represent one type of edge in a graph. A heterogeneous graph returns a list of multiple Edgeattrs, and a homogeneous graph returns a list containing one EdgeAttr.
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.
seems like a replica of the original client.yaml?
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.
Yes, this is the file I used to test FeatureStore and GraphStore in k8s, but the file name of the client code(PyG_client.py) has been changed.
from graphscope.learning.pyg_neighbor_sampler import PygNeighborSampler | ||
|
||
|
||
class GLTNeighborLoader(NodeLoader): |
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.
what is the difference between GLTNeighborLoader and NeighborLoader in PyG besides asserting neighbor_sampler is not None? or, can we just use NeighborLoader in PyG
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.
NeighborLoader PyG originally in iteration will call filter_fn () to convert the iterative output type (SamplerOutput/HeteroSamplerOutput - > Data/HeteroData), But the output of the PygNeighborSampler iteration is the Data/HeteroData type, so I've removed the parts related to filter_fn() here so that they can be properly connected.
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.
please follow python code style guide to rename your file name and function name like GSFeatureStore.py and PyG_remote_backend
Please check the preview of the documentation changes at |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4237 +/- ##
===========================================
+ Coverage 46.22% 60.99% +14.76%
===========================================
Files 174 126 -48
Lines 16181 13264 -2917
===========================================
+ Hits 7480 8090 +610
+ Misses 8701 5174 -3527
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 114 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
2617980
to
c065dd0
Compare
This reverts commit ce3cc2f.
8419914
to
bba3a8d
Compare
What do these changes do?
Step 1: Implement GraphScope-based PyG Remote Backend and complete the end-to-end integration of GraphScope and PyG. (Finished)
Step 2: Get data from the Server through PyG Remote Backend and support sampling on the Client side. (Finished)
Related issue number
PyG Remote Backend Based on GraphScope #3739