Skip to content
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

allow to pass unresolved loss function to ERModel's __init__ #717

Closed
mberr opened this issue Jan 7, 2022 · 4 comments · Fixed by #718
Closed

allow to pass unresolved loss function to ERModel's __init__ #717

mberr opened this issue Jan 7, 2022 · 4 comments · Fixed by #718

Comments

@mberr
Copy link
Member

mberr commented Jan 7, 2022

So then it's fine to finish this PR without adding a new model, but it would be important to write some narrative documentation explaining how one might use this interaction to construct a new model

This should be the place to put it? To slightly modify the example there and create an "anonymous" model instead of declaring a new class:

from pykeen.models import ERModel
from pykeen.nn import EmbeddingSpecification
model = ERModel(
    triples_factory=...,
    loss=..., # TODO: we should support passing a string here, and use the loss-resolver to resolve it
    interaction="transformer",
    entity_representations=EmbeddingSpecification(embedding_dim=64),
    relation_representations=EmbeddingSpecification(embedding_dim=64),
)

Originally posted by @mberr in #714 (comment)

@cthoyt
Copy link
Member

cthoyt commented Jan 7, 2022

Yeah, that's a great place for it!

@mberr
Copy link
Member Author

mberr commented Jan 7, 2022

We could also consider allowing to pass an integer as {entity,relation}_representations and create and internally create an EmbeddingSpecification with default settings from it. This might make it easier to use of ERModel via CLI. 🤔

@cthoyt
Copy link
Member

cthoyt commented Jan 7, 2022

You mean like if you pass an integer it just assumes you want a vanilla embedding spec with that as the dim? Or a sequence of integers would make a sequence of embeddings? Or a sequence of tuples of ints would make fun shaped ones?

@mberr
Copy link
Member Author

mberr commented Jan 7, 2022

You mean like if you pass an integer it just assumes you want a vanilla embedding spec with that as the dim? Or a sequence of integers would make a sequence of embeddings? Or a sequence of tuples of ints would make fun shaped ones?

Something like this. A shortcut which allows you to create something without having to import EmbeddingSpecification if you are not interested in all the possible configurations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants