Do not have one shared entity_embedder hard coded into KgeModel about kge HOT 6 CLOSED

Sure you can do it: the ComplEx model either takes the left (subject) or right (object) half of the entity embeddings.

from kge.

from kge.

Our philosophy should be to stay simple unless needed otherwise. The advantage of having a single embedder is that all our code can assume that there is just one entity embedding per entity. Only if there are real use-cases (CP isn't one, imo), we should add support. Otherwise we may end up with a large, messy implementation with lots of code that is not needed.

from kge.

If I understand correctly, this translates only to handling different embedders for subjects and objects by default. Then we should have that by default. It shouldn't be much change to leave this option open. The other way around, not having this and experimenting with models which require it, would imply more changes, which make experimentation more cumbersome.

But, if I don't understand correctly and it implies considerable changes to the current architecture, then no. Readability counts, I think, and having code which is considerably less readable due to features not many will need is not something we want.

from kge.

Ok, I've added this in the simplest possible way, i.e., I removed the hard-coded embedding references.

The idea is that by default KgeModel creates the embedders (not models), but models can overwrite as they please. The only diference between DistMult and CP should be a flag in the embedder configuration, but they can share the same implementation otherwise.

from kge.

Perfect! Thank you!

from kge.