[Cleanup] Gathering graph utilities around BasicGraph/MoleculeGraph #13
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Yoric
requested review from
RolandMacDoland and
ShaheenPasqal
and removed request for
RolandMacDoland and
ShaheenPasqal
There was a problem hiding this comment.
Thanks @Yoric. Few comments from my side. Happy to approve once addressed.
| if any(distance_from_center > device.max_radial_distance): | ||
| return False | ||
|
|
||
| # Check the distance between nodes. |
There was a problem hiding this comment.
Isn't it possible to reuse is_disk_graph here ?
There was a problem hiding this comment.
Well, in is_disk_graph there is this distance check between nodes. Just wondering if that couldn't be reused here.
Before this change, we have many graph-related functions with various pre-requisites on graphs, plus lots of conversions from pyg to networkx. This change: - introduces two classes BasicGraph and MoleculeGraph, depending on which invariants hold on the graph; - attaches the relevant methods to BasicGraph/MoleculeGraph instead of pyg; - makes sure that we have only one single conversion from pyg to networkx.
Furthermore, we move some code from the notebook to the library, as our users are going to need it.
RolandMacDoland
approved these changes
Jan 7, 2025
|
@Yoric Happy for you to merge once you've replied to the thread of discussion above. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We have many graph-related functions with various pre-requisites on graphs, plus lots of conversions from pyg to networkx.
This change: