search

allenai / deep_qa

A deep NLP library, based on Keras / tf, focused on question answering (but useful for other NLP too)
Apache License 2.0
404 stars 133 forks source link

cosine_similarity and also slot_similarity_tuple_matcher #228

Closed BeckySharp closed 7 years ago

BeckySharp commented 7 years ago

sure can :)

On Wed, Mar 1, 2017 at 11:28 AM, Matt Gardner notifications@github.com wrote:

@matt-gardner commented on this pull request.

In tests/layers/tuple_matchers/slot_similarity_tuple_matcher_test.py https://github.com/allenai/deep_qa/pull/228#discussion_r103768665:

  • hidden_layer_activation=self.hidden_layer_activation)
  • output = match_layer([self.tuple1_input, self.tuple2_input])
  • model = Model([self.tuple1_input, self.tuple2_input], output)

  • Get the initial weights for use in testing

  • dense_hidden_weights = K.eval(model.trainable_weights[0])
  • score_weights = K.eval(model.trainable_weights[1])

  • Testing general unmasked case.

  • similarity_function = CosineSimilarity(name="cosine_similarity")
  • cosine_similarities = similarity_function.compute_similarity(K.variable(self.tuple1), K.variable(self.tuple2))

  • Desired_overlap gets fed into the inner NN.

  • dense1_activation = numpy.dot(K.eval(cosine_similarities), dense_hidden_weights)
  • final_score = numpy.dot(dense1_activation, score_weights)
  • desired_result = logistic.cdf(final_score)

Oh, this is applying the sigmoid activation function? Can you add a comment saying that?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/allenai/deep_qa/pull/228#discussion_r103768665, or mute the thread https://github.com/notifications/unsubscribe-auth/AFIniQu7tXxtSEJPTD5DaxUxxBlQWmFrks5rhcb0gaJpZM4MN5rN .

BeckySharp commented 7 years ago

I can't until I have your changes to TimeDistributedWithMask. As it is now, I don't think it builds. If we do the merges, I can remove it and see if it works, and if so I'll remove it. Does that work?

On Wed, Mar 1, 2017 at 12:59 PM, Matt Gardner notifications@github.com wrote:

@matt-gardner commented on this pull request.

In deep_qa/layers/backend/squeeze.py https://github.com/allenai/deep_qa/pull/228#discussion_r103787259:

@@ -26,6 +26,11 @@ def compute_mask(self, inputs, mask=None):

pylint: disable=unused-argument

if mask is None: return None

  • If the mask doesn't need squeezing, just return it.

  • TODO(becky): better to check if the last mask dim == 1?

  • if K.ndim(inputs) == K.ndim(mask) + 1:

I'm pretty sure the shapes work out now that this isn't needed. Can you remove it and see if it still works?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/allenai/deep_qa/pull/228#discussion_r103787259, or mute the thread https://github.com/notifications/unsubscribe-auth/AFInifiv-wxuzN63IFZRV9V9R60LKTfsks5rhdw-gaJpZM4MN5rN .

matt-gardner commented 7 years ago

I just checked out the code to see if tests pass if I remove what you added to Squeeze, and they do. I would recommend removing it and adding it to the next PR, if it's still necessary.

BeckySharp commented 7 years ago

yeah, the tests only test the model not crashing with the WordOverlapTupleMatcher, duh. so tests pass. ok, will remove

On Wed, Mar 1, 2017 at 3:09 PM, Matt Gardner notifications@github.com wrote:

I just checked out the code to see if tests pass if I remove what you added to Squeeze, and they do. I would recommend removing it and adding it to the next PR, if it's still necessary.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/allenai/deep_qa/pull/228#issuecomment-283501655, or mute the thread https://github.com/notifications/unsubscribe-auth/AFIniXlbTmRxU-6DOLT6EX53-wI8o_lGks5rhfqngaJpZM4MN5rN .