-
Notifications
You must be signed in to change notification settings - Fork 1
Fix to make Directed graph to Undirected #7
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
base: dev
Are you sure you want to change the base?
Conversation
Tried running the normal GNN model with undirected graph and looks like for undirected graph the edge attributes needs to twice its number to able to be mapped to undirected edge (which is present as directed in both direction in https://wandb.ai/chebai/chebai/runs/fu1tcxf4/logs [rank0]: return self._call_impl(*args, **kwargs)
[rank0]: File "/home/staff/a/akhedekar/miniconda3/envs/gnn/lib/python3.10/site-packages/torch/nn/modules/module.py", line 1750, in _call_impl
[rank0]: return forward_call(*args, **kwargs)
[rank0]: File "/home/staff/a/akhedekar/miniconda3/envs/gnn/lib/python3.10/site-packages/torch_geometric/nn/conv/res_gated_graph_conv.py", line 128, in forward
[rank0]: out = self.propagate(edge_index, k=k, q=q, v=v, edge_attr=edge_attr)
[rank0]: File "/home/staff/a/akhedekar/atmp_dir/torch_geometric.nn.conv.res_gated_graph_conv_ResGatedGraphConv_propagate_x5frmxhf.py", line 231, in propagate
[rank0]: out = self.message(
[rank0]: File "/home/staff/a/akhedekar/miniconda3/envs/gnn/lib/python3.10/site-packages/torch_geometric/nn/conv/res_gated_graph_conv.py", line 144, in message
[rank0]: k_i = self.lin_key(torch.cat([k_i, edge_attr], dim=-1))
[rank0]: RuntimeError: Sizes of tensors must match except in dimension 1. Expected size 2438 but got size 1219 for tensor number 1 in the list.
|
- instead of using adjacent directed edge, this one is better approach since we can stack edge attributes generated later without any further logic to rearrange edge_attr
Training has been started for this change : https://wandb.ai/chebai/chebai/runs/9xjpb6wi?nw=nwuseraditya0by0 Another job started with 2 gpus: https://wandb.ai/chebai/chebai/runs/ejg3ksex?nw=nwuseraditya0by0 |
The training seems to be quite slow. I'm wondering if all of the following properties were actually used in the original training setup. Could you please share the corresponding Weights & Biases (wandb) link for the original run? Encoding lengths are:
[('AtomAromaticity', 1),
('AtomCharge', 13),
('AtomHybridization', 7),
('AtomNumHs', 7),
('AtomType', 119),
('BondAromaticity', 1),
('BondInRing', 1),
('BondType', 5),
('NumAtomBonds', 11),
('RDKit2DNormalized', 200)] |
Hi, I can confirm that the properties were used in actual runs, e.g. this one: https://wandb.ai/chebai/chebai/runs/cxmgl4eb (the technical setup is not the same one we use now, making it hard to compare, but I would expect it to get better with our current setup, not worse). The bottleneck for this model is the creation of the dataset (especially RDKit2DNormalized). But one that is done, I would expect normal-ish behaviour during training. |
instead of Base data module, as `load_processed_data_from_file` method used in this class is available in Dynamic dataset class
@sfluegel05, Please find the training below for this fix. Please review and merge. |
I'm not sure what the run is telling me. Training seems to be doing fine, but the macro-f1 is a few percent lower compared to https://wandb.ai/chebai/chebai/runs/0oksfx9u Does that mean that undirected is worse than directed? Or am I missing something here? |
Also, for your run, have you changed the
|
I have been using the default precision for my run as well ( |
Some of the code repository of the corresponding research papers which used undirected graph for chemical bonds. |
Directed: https://wandb.ai/chebai/chebai/runs/5yhpkxci/overview
|
I repeated the training on the same GPU type after making the training deterministic, and the results are consistent with the earlier observation: Undirected Graph (Deterministic runs): Directed Graph (Deterministic runs): |
_read_data
ofGraphPropertyReader
#6