Add ConvE scoring function #35
Conversation
There was a problem hiding this comment.
Look all good to me!
Regrading corrupted heads: I guess when corrupting only tails the weights in the conv/dense layer just learn some interaction between actual head/relation pairs. When corrupted heads come into the mix they have to learn to handle pairs of head/relation that don't occur together. Also, the batch norm across both, true and false pairs feels a bit weird (we might normalise them individually?) In the end I think we should just discourage using corrupted heads, maybe even add an exception in that case
besskge/scoring.py
Outdated
| tail_emb, tail_bias = torch.split(tail_emb, self.embedding_size, dim=-1) | ||
| hr_cat = torch.cat( | ||
| [ | ||
| head_emb.view(-1, self.inp_channels, self.emb_w, self.emb_h), |
There was a problem hiding this comment.
non-substantial, but maybe worth changing for comparability: pykeen reshapes to [-1, channels, height, width] and concats along height
There was a problem hiding this comment.
Thanks, I switched the role of height and width to align with PyKeen
Thanks for the review! I agree that batchnorm is doing something different in the two cases (score_heads vs score_tails). I decided to just remove the the score_heads methods altogether so that it throws an exception if someone tries to use ConvE with corrupt heads, what do you think? |
I agree, that's probably the best solution. |
Paper: https://cdn.aaai.org/ojs/11573/11573-13-15101-1-2-20201228.pdf
Original implementation: https://github.com/TimDettmers/ConvE/blob/master/model.py#L79
PyKeen: https://github.com/pykeen/pykeen/blob/master/src/pykeen/models/unimodal/conv_e.py, https://github.com/pykeen/pykeen/blob/master/src/pykeen/nn/functional.py#L99
The implementation follows quite closely the PK one. It's worth noting that, by how it's defined, the role of heads and tails in the scoring function is quite asymmetrical (heads and relations are combined via convolution, and the output is then multiplied with tails). Looking at the paper, this seems to be done by design to allow for efficient 1-N scoring (i.e. broadcasted scoring) of (h,r) queries against multiple tails at once. As a matter of fact, it was rather painless to implement the
score_tailsmethod using the broadcasted dot product that we already had for DistMult/ComplEx. However, there is no way of doing this forscore_heads, where we need to materialize all (bs, n_negative, emb_size) hr queries explicitly - and this is quite problematic when using negative sample sharing. I think it should only be used with "tail" corruption scheme, also because it does not seem to train correctly when adding head-corrupted triples (something that I am also seeing in PyKeen and that I am not sure I understand)...