Implement sample GAT model for working PyG with DeepChem #2109

nissy-dev · 2020-08-21T04:34:47Z

This PR is a part of #1942

What I did

Create the new molecule graph convolution featurizer using GraphData.
- The new featurizer return a basic atom features and bond features like GCN, MPNN.
  - The present graph conv features have some values of nodes and edges which the paper like WeaveNet, MEGNet, MPNN didn't mention
- Create utils/graph_conv_utils.py and the featurizer is more readable and customizable for users
Create a sample GAT model
- This model is an example of working DeepChem with PyTorch Geometric

TODO

Add more docstrings
Add more tests (mainly, all functions in utils/graph_conv_utils.py)

I will make another PR.

Debug the problem that my GAT model show the too low score of the overfit tests compared with present TF GraphConvModel
Add mode args for GATModel like Add mode args for CGCNNModel #2123

nissy-dev · 2020-08-24T15:50:14Z

This PR is ready to review!

rbharath

This is really neat! Well written, well documented code :)

I've done a first pass with a couple of comments

rbharath · 2020-08-25T03:23:22Z

deepchem/feat/molecule_featurizers/mol_graph_conv_featurizer.py

+  get_bond_stereo_one_hot
+
+
+def constrcut_atom_feature(atom: RDKitAtom, h_bond_infos: List[Tuple[int, str]],


typo: Should be construct_atom_feature.

rbharath · 2020-08-25T03:23:42Z

deepchem/feat/molecule_featurizers/mol_graph_conv_featurizer.py

+
+
+def constrcut_atom_feature(atom: RDKitAtom, h_bond_infos: List[Tuple[int, str]],
+                           sssr: List[Sequence]) -> List[float]:


Should we perhaps make this return an numpy array instead of a list?

Is there any reasons? I think this function are basically not used in other place. (Currently, this I added the underscore to this function name like _ construct_atom_feature )

rbharath · 2020-08-25T03:24:18Z

deepchem/feat/molecule_featurizers/mol_graph_conv_featurizer.py

+class MolGraphConvFeaturizer(MolecularFeaturizer):
+  """This class is a featurizer of gerneral graph convolution networks for molecules.
+
+  The default node(atom) and edge(bond) representations are based on WeaveNet paper.


Perhaps add a hyperlink to the weave paper in the references section below

rbharath · 2020-08-25T03:28:32Z

deepchem/feat/molecule_featurizers/mol_graph_conv_featurizer.py

+  The default node(atom) and edge(bond) representations are based on WeaveNet paper.
+  If you want to use your own representations, you could use this class as a guide
+  to define your original Featurizer. In many cases, it's enough to modify return values of
+  `constrcut_atom_feature` or `constrcut_bond_feature`.


Typo: Should be construct for both of these

coveralls · 2020-08-25T15:47:52Z

Coverage increased (+0.4%) to 77.861% when pulling b7b56fa on nd-02110114:gat-pyg-2 into bc05e7c on deepchem:master.

rbharath · 2020-08-25T17:50:55Z

One more quick ask, could you update the model cheatsheet to add GATModel and CGCNNModel? https://deepchem.readthedocs.io/en/latest/models.html#model-cheatsheet

nissy-dev · 2020-08-27T14:12:58Z

One more quick ask, could you update the model cheatsheet to add GATModel and CGCNNModel? https://deepchem.readthedocs.io/en/latest/models.html#model-cheatsheet

I will add a lot of documentations related to TorchModel, CGCNN, GAT in #2124

peastman · 2020-08-27T18:27:01Z

deepchem/feat/molecule_featurizers/mol_graph_conv_featurizer.py

+  - Chirality: A one-hot vector of the chirality, "R" or "S".
+  - Formal charge: Integer electronic charge.
+  - Partial charge: Calculated partial charge.
+  - Ring sizes: A one-hot vector of the number of rings (3-8) that include this atom.


I think you mean the size of the ring? Not many atoms belong to three rings, much less eight!

Yes! I fixed.

peastman · 2020-08-27T18:29:50Z

deepchem/feat/molecule_featurizers/mol_graph_conv_featurizer.py

+    """
+    Parameters
+    ----------
+    add_self_loop: bool, default False


Perhaps add_self_edges would be clearer? This isn't really about loops, so the name could be confusing.

I agreed! I fixed.

peastman · 2020-08-27T18:36:34Z

deepchem/feat/molecule_featurizers/mol_graph_conv_featurizer.py

+    )
+
+    # construct edge (bond) information
+    src, dist, bond_features = [], [], []


You probably want the variable to be called dest (short for destination?), not dist which sounds like it's short for "distance"?

This is short for destination. I fixed!

peastman · 2020-08-27T19:33:05Z

deepchem/models/torch_models/gat.py

+  >> dataset_config = {"reload": False, "featurizer": featurizer, "transformers": []}
+  >> tasks, datasets, transformers = dc.molnet.load_tox21(**dataset_config)


Wouldn't it be simpler to just write this as

tasks, datasets, transformers = dc.molnet.load_tox21(reload=False, featurizer=featurizer, transformers=[])

peastman · 2020-08-27T19:35:27Z

deepchem/models/torch_models/gat.py

+               in_node_dim: int = 38,
+               hidden_node_dim: int = 64,
+               heads: int = 4,
+               dropout_rate: float = 0.0,


For consistency with other models, this should be just dropout.

peastman · 2020-08-27T19:36:12Z

deepchem/models/torch_models/gat.py

+               heads: int = 4,
+               dropout_rate: float = 0.0,
+               num_conv: int = 3,
+               predicator_hidden_feats: int = 32,


Was that supposed to be "predictor"?

Yes. I fixed!

peastman · 2020-08-27T21:46:47Z

deepchem/utils/graph_conv_utils.py

@@ -0,0 +1,512 @@
+"""


This file doesn't really have anything to do with convolutions. How about calling it molecule_feature_utils.py?

nissy-dev · 2020-08-30T08:29:57Z

deepchem/feat/base_classes.py

+      args_info += arg_name + '=' + str(self.__dict__[arg_name]) + ', '
+    return self.__class__.__name__ + '[' + args_info[:-2] + ']'
+
+  def __str__(self) -> str:


This implementation is needed to resolve the Windows CI. This is referred #1829. repr function shows all arguments when instantiating a class. str function shows just updated arguments when instantiating a class.

nissy-dev · 2020-08-30T08:30:42Z

This PR is ready to a second review

rbharath

This is looking good! I have a couple minor comments. Once those are merged, I think this is good to merge in

deepchem/feat/base_classes.py

deepchem/feat/molecule_featurizers/mol_graph_conv_featurizer.py

rbharath

Looks good! Feel free to merge in whenever ready :)

nissy-dev · 2020-09-02T01:59:57Z

Thanks! I merge in

nissy-dev added 2 commits August 21, 2020 11:03

♻️ add new featurizer

3892a54

✨ add gat models

b26de50

nissy-dev changed the title ~~[WIP] Implement sample GAT models for working PyG with DeepChem~~ [WIP] Implement sample GAT model for working PyG with DeepChem Aug 21, 2020

✨ add docstrings and test

1a7996a

nissy-dev changed the title ~~[WIP] Implement sample GAT model for working PyG with DeepChem~~ Implement sample GAT model for working PyG with DeepChem Aug 24, 2020

✨ add doctsrings

c02779b

rbharath reviewed Aug 25, 2020

View reviewed changes

👌 fix typo and update docstrings

313d3a6

Merge branch 'master' into gat-pyg-2

00fc4ed

nissy-dev added 2 commits August 27, 2020 23:40

♻️ refactor function name

5870f9a

🚨 fix lint error

47835b4

peastman reviewed Aug 27, 2020

View reviewed changes

nissy-dev added 5 commits August 28, 2020 10:57

👌 update for review comments

1ef32ec

🚨 fix lint error

90f13f6

Merge remote-tracking branch 'upstream/master' into gat-pyg-2

f01a800

✨ add pretty print function

921c8c1

✨ add custom repr functions

97f5d2d

nissy-dev commented Aug 30, 2020

View reviewed changes

rbharath reviewed Sep 1, 2020

View reviewed changes

deepchem/feat/base_classes.py Show resolved Hide resolved

deepchem/feat/molecule_featurizers/mol_graph_conv_featurizer.py Outdated Show resolved Hide resolved

nissy-dev added 2 commits September 1, 2020 10:42

🐛 fix typo

b504f62

🐛 fix gpu bug

60a12f6

rbharath approved these changes Sep 1, 2020

View reviewed changes

♻️ fix default atom type

b7b56fa

nissy-dev merged commit 3d257a0 into deepchem:master Sep 2, 2020

nissy-dev deleted the gat-pyg-2 branch September 3, 2020 11:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement sample GAT model for working PyG with DeepChem #2109

Implement sample GAT model for working PyG with DeepChem #2109

nissy-dev commented Aug 21, 2020 •

edited

nissy-dev commented Aug 24, 2020

rbharath left a comment

rbharath Aug 25, 2020

rbharath Aug 25, 2020

nissy-dev Aug 27, 2020

rbharath Aug 25, 2020

rbharath Aug 25, 2020

coveralls commented Aug 25, 2020 •

edited

rbharath commented Aug 25, 2020

nissy-dev commented Aug 27, 2020

peastman Aug 27, 2020

nissy-dev Aug 28, 2020

peastman Aug 27, 2020

nissy-dev Aug 28, 2020

peastman Aug 27, 2020

nissy-dev Aug 28, 2020

peastman Aug 27, 2020

nissy-dev Aug 28, 2020

peastman Aug 27, 2020

nissy-dev Aug 28, 2020

peastman Aug 27, 2020

nissy-dev Aug 28, 2020

peastman Aug 27, 2020

nissy-dev Aug 28, 2020

nissy-dev Aug 30, 2020

nissy-dev commented Aug 30, 2020

rbharath left a comment

rbharath left a comment

nissy-dev commented Sep 2, 2020

		get_bond_stereo_one_hot


		def constrcut_atom_feature(atom: RDKitAtom, h_bond_infos: List[Tuple[int, str]],



		def constrcut_atom_feature(atom: RDKitAtom, h_bond_infos: List[Tuple[int, str]],
		sssr: List[Sequence]) -> List[float]:

		>> dataset_config = {"reload": False, "featurizer": featurizer, "transformers": []}
		>> tasks, datasets, transformers = dc.molnet.load_tox21(**dataset_config)

Implement sample GAT model for working PyG with DeepChem #2109

Implement sample GAT model for working PyG with DeepChem #2109

Conversation

nissy-dev commented Aug 21, 2020 • edited

What I did

TODO

nissy-dev commented Aug 24, 2020

rbharath left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Aug 25, 2020 • edited

rbharath commented Aug 25, 2020

nissy-dev commented Aug 27, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nissy-dev commented Aug 30, 2020

rbharath left a comment

Choose a reason for hiding this comment

rbharath left a comment

Choose a reason for hiding this comment

nissy-dev commented Sep 2, 2020

nissy-dev commented Aug 21, 2020 •

edited

coveralls commented Aug 25, 2020 •

edited