Add dgl #18620
Add dgl #18620
Conversation
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
|
It's been a while and unsuccessful, but I also made an attempt at #12552 |
|
Hi! This is the friendly automated conda-forge-linting service. I wanted to let you know that I linted all conda-recipes in your PR ( Here's what I've got... For recipes/dgl:
|
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
Co-authored-by: Jaime Rodríguez-Guerra <[email protected]>
|
Here is a small dgl python test script that could make the build even more robust (can be added as a # Test from https://docs.dgl.ai/tutorials/models/1_gnn/1_gcn.html
import time
import numpy as np
import torch as th
import torch.nn as nn
import torch.nn.functional as F
import dgl
import dgl.function as fn
from dgl import DGLGraph
from dgl.data import CoraGraphDataset
gcn_msg = fn.copy_u(u="h", out="m")
gcn_reduce = fn.sum(msg="m", out="h")
class GCNLayer(nn.Module):
def __init__(self, in_feats, out_feats):
super(GCNLayer, self).__init__()
self.linear = nn.Linear(in_feats, out_feats)
def forward(self, g, feature):
# Creating a local scope so that all the stored ndata and edata
# (such as the `'h'` ndata below) are automatically popped out
# when the scope exits.
with g.local_scope():
g.ndata["h"] = feature
g.update_all(gcn_msg, gcn_reduce)
h = g.ndata["h"]
return self.linear(h)
class Net(nn.Module):
def __init__(self):
super(Net, self).__init__()
self.layer1 = GCNLayer(1433, 16)
self.layer2 = GCNLayer(16, 7)
def forward(self, g, features):
x = F.relu(self.layer1(g, features))
x = self.layer2(g, x)
return x
def load_cora_data():
dataset = CoraGraphDataset()
g = dataset[0]
features = g.ndata["feat"]
labels = g.ndata["label"]
train_mask = g.ndata["train_mask"]
test_mask = g.ndata["test_mask"]
return g, features, labels, train_mask, test_mask
def evaluate(model, g, features, labels, mask):
model.eval()
with th.no_grad():
logits = model(g, features)
logits = logits[mask]
labels = labels[mask]
_, indices = th.max(logits, dim=1)
correct = th.sum(indices == labels)
return correct.item() * 1.0 / len(labels)
def main():
net = Net()
g, features, labels, train_mask, test_mask = load_cora_data()
# Add edges between each node and itself to preserve old node representations
g.add_edges(g.nodes(), g.nodes())
optimizer = th.optim.Adam(net.parameters(), lr=1e-2)
dur = []
t0 = time.time()
for epoch in range(50):
if epoch >= 3:
t0 = time.time()
net.train()
logits = net(g, features)
logp = F.log_softmax(logits, 1)
loss = F.nll_loss(logp[train_mask], labels[train_mask])
optimizer.zero_grad()
loss.backward()
optimizer.step()
if epoch >= 3:
dur.append(time.time() - t0)
acc = evaluate(net, g, features, labels, test_mask)
print(
"Epoch {:05d} | Loss {:.4f} | Test Acc {:.4f} | Time(s) {:.4f}".format(
epoch, loss.item(), acc, np.mean(dur)
)
)
if __name__ == "__main__":
main() |
|
@hadim Thanks! This is a good idea, I will admit that I'm not much of a I've uploaded the cuda 10.2 build, the others failed because of diskspace issues, if you want to test it you can use |
|
Yes, the code is device-agnostic. I don't know what is the recommended procedure here due to the disk space. Maybe simply merging as it is and see whether the feedstock CI pass on the failing cuda builds? @jaimergp @conda-forge/core ? |
|
running out of diskspace is "normal" for these kinds of builds and will be fixed/fine when it is in a feedstock. There are a few issues with the recipe that I need to fix, I was trying to first get something working before I made it better:
So quite of work still to do, but this looks like it is all tractable! Do you know @hadim if we can get upstream on-board with this at all? I know they have their own conda package on Some of these I guess should be done once we move it into a feedstock, but I think fixing vendored packages + using upstream should be done first IMHO. |
Definitely, that's the priority right now, I'd say. Without that it's very unlikely this recipe can be recommended for merge. CPU, non-Linux stuff can be worked on after that. Can you move that check list in the description of the PR @mikemhenry? It'll make following the progress easier. |
|
I agree there is more to be done here (I haven't noticed you weren't building for osx and win). I see you already commented on the ticket I opened a while ago @mikemhenry at dmlc/dgl#1855. I will ping the dgl folks there again to see if they help here. |
Co-authored-by: Jaime Rodríguez-Guerra <[email protected]>
Thanks! Header only libraries are fine to vendor (we just need to make sure we package the license) but it would be great to have their help in modifying the build system to use installed packages (I will admit that I've only looked at the CMakeLists.txt and it looks like there isn't an option to use installed libraries but I am not a CMake wizard) |
|
Hey @mikemhenry, I am just checking whether you are still planning to work on that PR? |
|
@hadim I am -- but do you have any cmake experience? I've been busy with some other projects and haven't gotten to it, but I'm guessing it would be quick for someone who is proficient with cmake. |
|
Thanks @mikemhenry. This is not urgent on my side, and I am quite busy as well, but I just wanted to know whether it was still on your radar or not. My cmake skills are quite old at this point to be honest xD If you're too busy in the future, I might give it a try at some point but I really don't know when. |
|
@hadim made a wonderful table here: dmlc/dgl#1855 (comment) outlining which packages need to get onto conda-forge and which ones already exist. I am going to try and make the changes needed upstream, but I might use a patch if I get things working and upstream wants to take their time in evaluating it (which is their right 😄 ) |
|
we will get DGL on conda-forge here: #22691 |
Checklist
url) rather than a repo (e.g.git_url) is used in your recipe (see here for more details).There are a few issues with the recipe that I need to fix, I was trying to first get something working before I made it better:
- [ ] Build for different OS, right now it is linux only- [ ] Build a CPU version for people who don't want to pull in CUDAThe crossed out TODOs will be done once we get this moved into a feedstock.
Supersedes #16924
Thanks @knc6 for starting this work!