-
-
Notifications
You must be signed in to change notification settings - Fork 179
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
🌯 🌪️ Encapsulate inverse relations #752
base: master
Are you sure you want to change the base?
Conversation
What's the roadmap for this PR? I think that it's fine to keep |
…-relations # Conflicts: # src/pykeen/datasets/ogb.py # src/pykeen/models/base.py # src/pykeen/models/nbase.py # src/pykeen/training/lcwa.py
…nto encapsulate-inverse-relations # Conflicts: # src/pykeen/training/lcwa.py
…nto encapsulate-inverse-relations
src/pykeen/models/nbase.py
Outdated
@@ -339,6 +339,11 @@ def __init__( | |||
triples_factory=triples_factory, | |||
representations=relation_representations, | |||
representations_kwargs=relation_representations_kwargs, | |||
# TODO: check this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: check this
@@ -651,10 +657,13 @@ def _get_representations( | |||
h: Optional[torch.LongTensor], | |||
r: Optional[torch.LongTensor], | |||
t: Optional[torch.LongTensor], | |||
invert_relation: bool = False, # TODO: do we need this here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: do we need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would be necessary if we had a separate Representation module for inverse relations 🤔
elif self.target == 1: | ||
self.score_method = self.model.score_r # type: ignore | ||
elif self.target == 2: | ||
self.supports_slicing = self.model.can_slice_r | ||
elif self.target == 2: # TODO: inverse relations? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO what about inverse relations?
Fixes #749
Tasks:
inverse_triples
flags fromTriplesFactory
andDatasets
.Dependencies
New Structure
score_hrt(h, r, t)
score_h(r, t, slice_size)
score_r(h, t, slice_size)
score_t(h, r, slice_size)
score_hrt_extended(h, r, t, invert_relation)
score_h_extended(r, t, slice_size, invert_relation)
score_r_extended(h, t, slice_size, invert_relation)
score_t_extended(h, r, slice_size, invert_relation)
score_h
):predict_hrt(h, r, t, invert_relation)
predict_h(r, t, slice_size, invert_relation)
predict_r(h, t, slice_size, invert_relation)
predict_t(h, r, slice_size, invert_relation)
score_{hrt,h,t}_inverse
-> move to prediction workflows?