tensorflow / models

Models and examples built with TensorFlow
Other
77.16k stars 45.76k forks source link

Probably a mistake in neumf_model.py evaluation #10305

Closed poorjin closed 1 year ago

poorjin commented 3 years ago

Prerequisites

Please answer the following question for yourself before submitting an issue.

1. The entire URL of the documentation with the issue

https://github.com/tensorflow/models/blob/master/official/recommendation/neumf_model.py#L423-L429

2. Describe the issue

In line 397, the args description of compute_top_k_and_ndcg() says,

logits ... Logits for a user are grouped, and the first element of the group is the true element.

However, the code below has

  # Determine the location of the first element in each row after the elements
  # are sorted.
  sort_indices = tf.argsort(logits_by_user, axis=1, direction="DESCENDING")

  # Use matrix multiplication to extract the position of the true item from the
  # tensor of sorted indices. This approach is chosen because both GPUs and TPUs
  # perform matrix multiplications very quickly. This is similar to np.argwhere.
  # However this is a special case because the target will only appear in
  # sort_indices once.
  one_hot_position = tf.cast(
      tf.equal(sort_indices, rconst.NUM_EVAL_NEGATIVES), tf.int32)

Isn't tf.equal(sort_indices, rconst.NUM_EVAL_NEGATIVES), tf.int32) actually looks for the last element instead of the first element, which is supposed to be the positive interaction?

Shouldn't that part be tf.equal(sort_indices, 0, tf.int32)?

There is also another description says,

logits... Logits for a user are grouped, and the last element of the group is the true element.

These two descriptions seem like two contradictory things since one claims the last element is the true element while the other one claims the first element, but actually look for the last element in the code below it.

laxmareddyp commented 1 year ago

Hi @poorjin,

Closing this issue as the code is deprecated and we are not using anymore.Please let us know if you need to discuss or add any comments here , feel free to reopen the issue.

Thanks.