ENMF shall compute over batch of user instead of full users

[rowedenny]

RecBole/recbole/model/general_recommender/enmf.py

Line 85 in f56bcd0

user_sum = torch.bmm(self.user_embedding.weight.unsqueeze(2),

Refer to the ENMF paper, the loss always goes with a batch of users. The score from function forward comes from users in current interaction, I assume it would be the same when computing user_sum, not all users.

[linzihan-backforward]

@rowedenny Thanks for your attention ! ENMF is a sepcific model that different from other gereral recommender models.
Its loss contains two part, the first part is calculated on all user and all item, and the second part is calculated on batch user.
You can refer to the Tensorflow source code of ENMF.
https://github.com/chenchongthu/ENMF/blob/a3b186afca8f5ee36cf893734b4bbb55faa55a7a/code/ENMF.py#L98

[rowedenny]

Hi, thanks for your quick response. However, I am afraid that I cannot agree with you. We can investigate it from two aspects:

1. In ENMF paper, we can see clues from Equation 11. The loss consists of two terms, one comes from observed data, and the other one comes from all data. Note that here, both terms are summed over user u in a batch of users. So I assume they shall be consistent, i.e., both are computed over a batch of users.
   

2. Also notice in the TF code, self.uid actually comes from self.uid = tf.nn.embedding_lookup(self.uidW, self.input_u) https://github.com/chenchongthu/ENMF/blob/a3b186afca8f5ee36cf893734b4bbb55faa55a7a/code/ENMF.py#L77.
   In addition, the placehold self.input_u is fed with a batch of users, please see, https://github.com/chenchongthu/ENMF/blob/a3b186afca8f5ee36cf893734b4bbb55faa55a7a/code/ENMF.py#L254.

From these two aspects, I believe the current implementation has a minor issue, though I am not sure if one term is computed for full batch, while the other for a batch could result in loss inblance. Correct me if I have any misunderstandings.

[linzihan-backforward]

Hi @rowedenny. After consideration, i think you are right. It is exactly a BUG of our implementation. We will fix it as soon as possiable. Thank again for your carefully review of the code.

[rowedenny]

If my understanding is correct, we only need to update the user_sum by computing it over the batch of users from interactions, other things should be all the same.

Is that correct?

[rowedenny]

By the way, I may also point out one minor issue

RecBole/recbole/model/general_recommender/enmf.py

Line 49 in f56bcd0

self.apply(xavier_normal_initialization)

Recall the padding item is zeros, however, the xavier_uniform_initialization initialization initializes all the embeddings, including the padding_idx. Therefore when we compute the loss with respect to L_A, this non-zero vector will also contribute to loss unexpectedly in the following places:

RecBole/recbole/model/general_recommender/enmf.py

Line 68 in f56bcd0

item_embedding = self.item_embedding(user_inter) # shape: [B, max_len, embedding_size]

RecBole/recbole/model/general_recommender/enmf.py

Line 81 in f56bcd0

item_sum = torch.bmm(self.item_embedding.weight.unsqueeze(2),

Unless we use a mask to mask out the padding influence.

In short, I assume it is more appropriate to remove the xavier_normal_initialization initialization.