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

Make an object-oriented interface for embedding models #22

Open
auroracramer opened this issue Mar 20, 2019 · 4 comments
Open

Make an object-oriented interface for embedding models #22

auroracramer opened this issue Mar 20, 2019 · 4 comments
Labels
enhancement New feature or request

Comments

@auroracramer
Copy link
Collaborator

An object interface for extracting embeddings would make it simpler to extract multiple embeddings and would allow for the embedding model to just be loaded once instead of every time the get_embedding function is called.

At least to begin with, the interface could just be something like

m = EmbeddingModel(input_repr="mel256", content_type="music", 
                                    embedding_size=6144, center=True, hop_size=0.1):
emb, ts = m.get_embedding(audio, sr, verbose=1)
m.process_file(filepath, output_dir=None, suffix=None)

Basically, we'd just put all of the functions from https://github.com/marl/openl3/blob/master/openl3/core.py into a class.

@auroracramer auroracramer added the enhancement New feature or request label Mar 20, 2019
@justinsalamon
Copy link
Collaborator

Sounds reasonable. Perhaps I'd move the hop_size and embedding_size params into the get_embedding function call, as these are independent of the specific model file loaded?

@justinsalamon
Copy link
Collaborator

Do we still want to do this now that you can pass a model to get_embedding?

@auroracramer
Copy link
Collaborator Author

I think it might still be a decent idea, but as of now there's no rush towards an OOP model. The main benefit I suppose would be so that you don't have to pass around a model object. But on the other hand it's not really that much bookkeeping. And perhaps it would be best to err on the side of simplicity before overengineering an OOP API?

@justinsalamon
Copy link
Collaborator

justinsalamon commented May 13, 2019

Yeah could still be handy, let’s keep things stable for now and we can revisit this question down the line if/when we get more user feedback

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

No branches or pull requests

2 participants