Qcactus/add lightgcn

[Qcactus]

Description

Add LightGCN algorithm and lightgcn_deep_dive notebook.

Related Issues

Checklist:

• I have followed the contribution guidelines and code style for this project.
• I have added tests covering my contributions.
• I have updated the documentation accordingly.
• This PR is being made to staging and not master.

[review-notebook-app]

Check out this pull request on 

Review Jupyter notebook visual diffs & provide feedback on notebooks.

---

Powered by ReviewNB

[ghost]

All CLA requirements met.

[Leavingseason]

Hi all, this is a contributor from university. They are putting the very recent SIGIR2020 paper https://arxiv.org/abs/2002.02126 to our codebase. I will go through the code first and will let the authors correct some issues (if there is any). After that I will ping you in Teams so that you can start the review process.

[miguelgfierro]

this is awesome! super good work

[miguelgfierro]

for all the deprecated warnings:

WARNING:tensorflow:From ../../reco_utils/recommender/deeprec/graphrec/lightgcn.py:158: The name tf.sparse_tensor_dense_matmul is deprecated. Please use tf.sparse.sparse_dense_matmul instead. 
WARNING:tensorflow:From ../../reco_utils/recommender/deeprec/graphrec/lightgcn.py:116: The name tf.train.AdamOptimizer is deprecated. Please use tf.compat.v1.train.AdamOptimizer instead.
WARNING:tensorflow:From ../../reco_utils/recommender/deeprec/graphrec/lightgcn.py:117: The name tf.train.Saver is deprecated. Please use tf.compat.v1.train.Saver instead.
WARNING:tensorflow:From ../../reco_utils/recommender/deeprec/graphrec/lightgcn.py:119: The name tf.GPUOptions is deprecated. Please use tf.compat.v1.GPUOptions instead.
WARNING:tensorflow:From ../../reco_utils/recommender/deeprec/graphrec/lightgcn.py:120: The name tf.Session is deprecated. Please use tf.compat.v1.Session instead.
WARNING:tensorflow:From ../../reco_utils/recommender/deeprec/graphrec/lightgcn.py:121: The name tf.ConfigProto is deprecated. Please use tf.compat.v1.ConfigProto instead.
WARNING:tensorflow:From ../../reco_utils/recommender/deeprec/graphrec/lightgcn.py:123: The name tf.global_variables_initializer is deprecated. Please use tf.compat.v1.global_variables_initializer instead.

would you mind to change the code to tf.compact.v1? it would help if/when we change to TF2

Reply via ReviewNB

[Leavingseason]

Do we have the plan to switch to TF2?
If not, I should suggest it keep the old fashion, because as far as I know, most of people in industry are not willing to use TF2, becuase it will cause a lot of platform refactor in their codebase,

[miguelgfierro]

so far we are not planning to, there was a discussion about this #953.

However, I guess that at some point we will change, but I don't expect us to rewrite the algos for TF2. We are doing a large refactor in PR #1086 and one of the things we are doing, slowly, is to add tf.compat.v1 https://github.com/microsoft/recommenders/blob/24b6ba9664b808abb41f118c9adefb983b56be1d/reco_utils/recommender/ncf/ncf_singlenode.py#L58. The reason to do this work now is to save work in the future, if at some point we change to TF2, if we have everything changed to compat.v1, we won't break the repo.

[anargyri]

Great work, thanks for contributing this method!

[miguelgfierro]

the code is really nice, but there are several parts that I think we should change before merging. I think it is important to have single responsibility with the fit method and also to perform DRY in the metrics

[miguelgfierro]

this is really good

[miguelgfierro]

hey @Qcactus, this is really nice. If you want to, feel free to add your name to https://github.com/microsoft/recommenders/blob/master/AUTHORS.md

[miguelgfierro]

hey @Qcactus, in which machine have you computed the stats of the notebook? I'm trying to replicate

FYI @Leavingseason

[Qcactus]

@miguelgfierro GeForce GTX 1080Ti. Anything wrong with the notebook?

[miguelgfierro]

@miguelgfierro GeForce GTX 1080Ti. Anything wrong with the notebook?

I was testing the notebook on a K80 gpu with different batch sizes, but interestingly, the gpu memory doesn't change when I show it with nvidia-smi. These are the results I got:

EPOCHS = 5
#BATCH_SIZE = 1024 # with ML1m: Epoch 2 (train)169.7s, gpu memory 56Mb
#BATCH_SIZE = 4096 # with ML1m: Epoch 2 (train)47.2s, gpu memory 56Mb
# BATCH_SIZE = 16384 # (=1024*4*4), with ML1m: Epoch 2 (train)16.2s, gpu memory 56Mb
# BATCH_SIZE = 65536 # (=1024*4*4*4), with ML1m: Epoch 2 (train)9.0s, gpu memory 56Mb

I tried another machine, this time with 4K80, and got similar results: BATCH_SIZE = 65536 # (=1024*4*4*4), with ML1m: Epoch 2 (train)8.8s, gpu memory 56Mb

Looking at the code it seems it is using gpu: https://github.com/microsoft/recommenders/blob/staging/reco_utils/recommender/deeprec/models/graphrec/lightgcn.py#L107 so I don't understand why the gpu memory is so low, do you know what could be happening?

[Qcactus]

@miguelgfierro
I tested the notebook on GeForce GTX 1080Ti. Here is the result:

MovieLens 1m
BATCH_SIZE=1024, Epoch 2 (train)28.0s, GPU Memory 396Mb
BATCH_SIZE=4096, Epoch 2 (train)13.3s, GPU Memory 396Mb
BATCH_SIZE=16384, Epoch 2 (train)9.7s, GPU Memory 396Mb
BATCH_SIZE=65536, Epoch 2 (train)7.5s, GPU Memory 457Mb

It seems reasonable on my machine. But I don't have access to other kinds of GPU, so I might not be able to find out the problem. Have you used tensorflow 1.15.2? (I noticed that some codes in this repo are tested with tf 1.11.) Or maybe you can try to test the notebook on a GeForce to see whether the result is similar with mine.

[miguelgfierro]

I found the issue, the low memory consumption we were having was because the datasets were small, if I used ML10M or ML20M, I was getting 6713MiB