-
Notifications
You must be signed in to change notification settings - Fork 2
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
Generative model #42
Generative model #42
Conversation
thanks a lot! this looks neat! I'll do line-by-line review in just a bit |
pinot/generative/torch_gvae/model.py
Outdated
self.gc3 = GraphConvolution(hidden_dim1, hidden_dim2, dropout, act=lambda x: x) | ||
self.dc = InnerProductDecoder(dropout, act=lambda x: x) | ||
|
||
def parameterization(self, x, adj): |
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.
I made this typo before. This should spell 'parametrization' without the extra e.
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.
This is really odd, this file is outdated. Now sure why it pops up in here. The GCNModelVAE should not even have this parametrization function
class GCNModelVAE(nn.Module):
"""Graph convolutional neural networks for VAE
"""
def __init__(self, input_feat_dim, hidden_dim1=32, \
hidden_dim2=32, hidden_dim3=16, dropout=0.1, \
log_lik_scale=1):
""" Construct a VAE with GCN
"""
super(GCNModelVAE, self).__init__()
self.linear = nn.Linear(input_feat_dim, hidden_dim1)
self.gc1 = GN(hidden_dim1, hidden_dim2)
# Mapping from "latent graph" to predictive distribution parameter
self.output_regression = nn.ModuleList([
GN(hidden_dim2, hidden_dim3),
GN(hidden_dim2, hidden_dim3),
])
# Decoder
self.dc = InnerProductDecoder(dropout)
# Relative weight between the KL divergence and the log likelihood term
self.log_lik_scale = log_lik_scale
def forward(self, g):
""" Compute the parameters of the approximate Gaussian posterior
distribution
Args:
x (FloatTensor): node features
Shape (N, D) where N is the number of nodes in the graph
adj (FloatTensor): adjacency matrix
Shape (N, N)
Returns:
z: (FloatTensor): the latent encodings of the nodes
Shape (N, hidden_dim2)
"""
z1 = self.linear(g.ndata["h"])
z = self.gc1(g, z1)
return z
def condition(self, g):
""" Compute the approximate Normal posterior distribution q(z|x, adj)
and associated parameters
"""
z = self.forward(g)
theta = [parameter.forward(g, z) for parameter in self.output_regression]
mu = theta[0]
logvar = theta[1]
distribution = torch.distributions.normal.Normal(
loc=theta[0],
scale=torch.exp(theta[1]))
return distribution, theta[0], theta[1]
gcn_reduce = fn.sum(msg='m', out='h') | ||
|
||
""" | ||
Standalone convolution layers, separate from representation.dgl_legacy |
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.
Could you comment on how is this different from the vanilla Kipf and Welling Graph Conv? is it the explicit dependency on adjacent matrix that's different?
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.
This has been removed from updated code
pinot/generative/torch_gvae/utils.py
Outdated
return train_data, test_data, val_data | ||
|
||
|
||
def prepare_train_test_val(data): |
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.
could you comment a bit on the necessity of this as opposed to partition / sample small graphs and then batch them together?
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.
I don't know the answer to this question yet. But updated code allows us to choose how many molecules get "batched" into a macromolecule per iteration
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.
Thanks a lot! This looks great.
Minor comments:
- I'm a bit confused by the necessity of doing all the operations based on adjacency matrix. it might not be the most efficient way and could be error prone.
- I'd suggest that we stick to torch-generic methods when possible
Major modifications compared to #34