Computation of penalty terms about kge HOT 11 CLOSED

What is the reasoning? They are used twice hence the penalty is computed for each factor matrix of each mode, which does matter in two cases: 1. when the penalty that is computed is weighted 2. when the penelty weight comes from a shared lookup_embedder config setting, such that we cannot learn/tune the scaling.

from kge.

I can see arguments for both ways of doing it.

1. Keep as before: Do it like everywhere else (all related work). More intuitive/natural. Better performance.

2. Change is in this commit: No special treatment of the shared_embedders case. Anything else?

The weights are not a big deal: can be done both ways: in (1) by simply passing along a list of ids (as suggested in #39), in (2) by calling penalty twice (for s, for o). (BTW: currently, the penalty function in KgeModel does not correctly pass along ids.)

from kge.

currently, the penalty function in KgeModel does not correctly pass along ids.

How is that?

from kge.

currently, the penalty function in KgeModel does not correctly pass along ids.

How is that?

All fine, I must have been looking at outdated code.

from kge.

The current implementation of these penalty terms for lookup_embedders uses embed:

parameters = self.embed(kwargs['batch']['triples'][:, kwargs['slot']])

This may be flawed since this will run dropout (but shouldn't).

from kge.

from kge.

Looks fine now. The only suggested change I still have is the API change from #39:

I suggest to change self.get_s_embedder().penalty(slot=0, **kwargs) to self.get_s_embedder().penalty(penality_ids=triples[slot], **kwargs) or so. This way, the embedder can be used later on when the weights do not come from triples.

from kge.

The change above is still open.

I also think that it would be helpful (for understanding the model) if the penalty terms were named (e.g., entity_embedder.l2) and traced/printed both with and without the regularization weight being applied.

from kge.

Is this currently being worked on already?

from kge.

Yes, by @Nzteb

from kge.

Closed with #101

from kge.