Rename inverse relations flag #1093
Conversation
mberr
changed the title
|
Are we sure this is worth the massive diff and backwards incompatibility? |
| model=model, | ||
| model_kwargs=dict(embedding_dim=embedding_dim, loss=loss), | ||
| model_kwargs=dict(embedding_dim=embedding_dim, loss=loss, use_inverse_relations=use_inverse_relations), |
There was a problem hiding this comment.
is this correct? shouldn't it still be in the dataset_kwargs before #752?
|
Originally I wasn't so keen on this because I was worried about backwards compatibility issues, but I think we can throw that to the wind. BUT, I still think there are a lot of spurious changes here that will just get deleted in the next PR, so can't we just do #752 first without the name change, then do a second (much smaller) PR after that updates the names? |
Yes, should be fine, too. We should remember to update the configs, too 🙂 We could add some logic to loading configurations to either map the old name to the new one, or warn about it linking to this issue / how to fix old configs. |
Cherry pick from #752 to reduce diff, as well as consistent renaming across all files.
note: the large number of changed files is partially caused by the experimental configurations
With the upcoming changes from #752, there will no longer be the need to create inverse triples, and only the relation representations will have the artificial relations introduced by inverse relations.