KGCN-pytorch
KGCN-pytorch copied to clipboard
Make several improvements
I have made several improvements and it would be an honor if you could review my code. Please feel free to decide whether to accept this pull request or not.
Comparison with PyTorch KGCN
Add a new handcrafted dataset product
- Dataset source: Rec-Tmall
- Preprocessing script: preprocessing.ipynb
Add a new mixer transe
transe
mixer has a better performace on divergence speed, time complexity, divergence loss value and AUC value, final-epoch loss value and AUC value in most cases.
See _mix_neighbor_vectors_TransE()
in aggregator.py
Add batch_experiments.ipynb
for convenient experiment
Automatically conduct multiple experiments.
- Config many parameter sets once
- Run
- Check results in a well-formatted print-out
Fix RuntimeError: indices should be either on cpu or on the same device as the indexed tensor (CPU)
Before
https://github.com/zzaebok/KGCN-pytorch/blob/3b0bb56da4b6759d204de06f1d4547e9b4abe3ce/model.py#L79-L80
After
neighbor_entities = torch.LongTensor(self.adj_ent[entities[h].cpu()]).view((self.batch_size, -1)).to(self.device)
neighbor_relations = torch.LongTensor(self.adj_rel[entities[h].cpu()]).view((self.batch_size, -1)).to(self.device)
Fix IndexError: index out of range in self
Before
https://github.com/zzaebok/KGCN-pytorch/blob/3b0bb56da4b6759d204de06f1d4547e9b4abe3ce/model.py#L34-L35
After
self.adj_ent = torch.zeros(self.num_ent, self.n_neighbor, dtype=torch.long)
self.adj_rel = torch.zeros(self.num_ent, self.n_neighbor, dtype=torch.long)
Improve readability
- Add more comments
- Reorganize some codes
- etc.
Hi,
Thank you for the valuable PR!
Before I review the code, let me emphasize what I consider important: the relevance of the original code(https://github.com/hwwang55/KGCN).
This repository is essentially a PyTorch implementation of KGCN.
Therefore, it should be helpful in understanding how KGCN works.
Consequently, I prefer simplicity and understandability over the performance of the implementation.
In this context, I really appreciate that you created a file like KGCN.py
to easily run the experiments.
Before I look into your code, I have some questions. Please take a look and let me know the answers.
-
You attached
.cpu()
to the tensors to fix theRuntimeError: indices should be either on cpu or on the same device as the indexed tensor (CPU)
. As far as I know, this feature moves tensors from GPU to CPU. I don't know which devices you used for training/evaluation, but usingcpu()
could be a performance issue for those who use GPU. Could you please provide more details? -
You changed
torch.empty
totorch.zeros
to fixIndexError: index out of range in self
. However, I cannot find information indicating that onlytorch.empty
can generate an index error. Can you please provide a reference?
For the first one, I do use cuda. I move the tensor to cpu because the error information tells me to do so. I also don't know why it says the indices and indexed tensors are on different devices.
For the second one, I suppose using torch.zeros
will be more robust because the 0-index will never cause an out-of-range error. Also, I don't know why torch.empty
doesn't work. Though it should work because we know the values inside the tensor will be replaced with valid indices.
I'm so sorry that my justifications are not detailed enough for I've just started learning those things.