search

tensorflow / neural-structured-learning

Training neural models with structured signals.
https://www.tensorflow.org/neural_structured_learning
Apache License 2.0
980 stars 189 forks source link

This line should the other way around. If weights is None then don't unpacked from the input #84

Closed basselkassem closed 3 years ago

basselkassem commented 3 years ago

https://github.com/tensorflow/neural-structured-learning/blob/c21dad4feff187cdec041a564193ea7b619b8906/neural_structured_learning/keras/layers/pairwise_distance.py#L114

aheydon-google commented 3 years ago

Hi, Bassel, thanks for the report.

This is a bit confusing, but I actually think the code is correct. The documentation says:

    Args:
      inputs: Symbolic inputs. Should be (sources, targets) if `weights` is
        non-symbolic. Otherwise, should be (sources, targets, weights).
      weights: If target weights are not symbolic, `weights` should be passed as
        a separate argument. In this case, `inputs` should have length 2.

The idea is that if you pass None for weights, the inputs argument is expected to be a tuple of 3 elements; otherwise, it's expected to be a tuple of 2 elements. And that's just what the code does:

    if weights is None:
      sources, targets, weights = inputs
    else:
      sources, targets = inputs

(Notice in the else case that the weights variable is left unchanged because it already has a non-None value.)

So I'm going to close this bug as I believe the code is correct. Please feel free to re-open it if you think I'm mistaken.

Thanks!

basselkassem commented 3 years ago

Thank you Aheydon for your replay. I get it know!