Skip to content
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

DeosGCF implementation #594

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

DeosGCF implementation #594

wants to merge 2 commits into from

Conversation

deklanw
Copy link
Contributor

@deklanw deklanw commented Dec 18, 2020

Paper here: https://arxiv.org/abs/2011.02100

TF implementation here: https://github.com/JimLiu96/DeosciRec

The algorithm is pretty similar to LightGCN so there wasn't too much to do. The paper claims an improvement over LightGCN, which I'm not seeing. So, there might be a mistake somewhere in my implementation.

Named DeosGCF because DGCF is already taken.

@deklanw
Copy link
Contributor Author

deklanw commented Dec 18, 2020

Btw, the utility functions here https://github.com/RUCAIBox/RecBole/pull/594/files#diff-f5377603dd836d2a11c0891c63600cfe4461a3a3e2deffd88ab05b234bd272daR11-R66 are probably worth extracting out. Some are used e.g. in the LightGCN implementation, but also SparseDropout for GCMC etc

@ShanleiMu
Copy link
Member

ShanleiMu commented Dec 19, 2020

Thanks for this implementation. We will review the code and test its performance on our benchmarks. If everything is OK, we will merge it.

@deklanw
Copy link
Contributor Author

deklanw commented Dec 19, 2020

Thanks. I assume those are internal benchmarks? If so, I'm curious what the results are

@ShanleiMu
Copy link
Member

@deklanw Thanks again for your nice contributions (#594, #609, #621, #634, #662, #670). Due to the busy schedule, I'm sorry to postpone reviewing and testing these codes.

We will complete the review and performance test before March, 1st and release them in the next version.

A = adjacency_of_bipartite(self.interaction_matrix)
norm_adj_matrix = get_symm_norm_tensor(A).to(self.device)
norm_crosshop_matrix = get_symm_norm_tensor(A**2).to(self.device)
self.A_hat = norm_adj_matrix + norm_crosshop_matrix
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the original paper, the matrix is the sum of three matrix: adjacent matrix, cross-hop matrix and identity matrix. It seems that this implementation lack the identity matrix.
paper

# generate intermediate data
A = adjacency_of_bipartite(self.interaction_matrix)
norm_adj_matrix = get_symm_norm_tensor(A).to(self.device)
norm_crosshop_matrix = get_symm_norm_tensor(A**2).to(self.device)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a high-pass filtering on the corss-hop metrix which greatly improves the efficiency. It should also be implemented.
paper1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants