Verify sparse regularization about kge HOT 11 CLOSED

M first implementation was doing the kw argument thing, which was exploding in the number of changes. I agree that it operates under the given assumption,. However, I am deliberately not using the embed_all func, which is why it currently works for 1toN.

from kge.

Ok, this needs revision + I think it's risky to use right now. Are you 100% it is correct for 1toN?

from kge.

It is not risky now. But It could be nicer done. As I said,I already had it working with the kwarg way, but it involved a lot of changes for which I wanted us to discuss it first, how we want to do it.

How it did it was that the penalty computation lived now in the KgeModel class and the penalty had a kwargs argument. The annoying thing is that this meant that we would have to change every forward of every model to correctly collect the sparsely embedded spo, sp or po embeddings. I did it for complex, but then figured that what I am doing now is equivalent, ss long as we only care about the lookupembedders parameters.

from kge.

I guess the nice solution would be if the the scorer collects all the parameters and its hyperparameters (reg weight + type), agreed?

from kge.

Are you 100% it is correct for 1toN?

No actually I have to fix it.

from kge.

I think the trainer should collect the ids of the entities and relations that are used in the batch and pass it to the penalty function. This way, the penalty function becomes trivial (like the code you quoted somewhere a few days ago) and different training jobs can collect these ids in the most suitable way.

from kge.

This would imply that the penalty function knows what the scorer is doing with those ids, whcih IMHO is as risky as the assumption I was using.

from kge.

Also, ideally we would design something that lets us also use the same penalty for non embedder parameters liek the hidden layers from NN models and such. This, however, would mean that we might have to move the penalty term into the model scope.

from kge.

The penalty term is already in the model scope. It's just that it forwards the computation of the embedder penalty terms to the embedders, but each model can add arbitrary other penalties if it wanted to do so.

from kge.

I think the revised implementation looks much better. It will also throw an error if the trainer is not setup correctly (passes the right information to the model). Is it implemented for both trainers already?

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.

I am closing this due to #41, ok?

from kge.