-
Notifications
You must be signed in to change notification settings - Fork 239
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
Support for Flair Embeddings: hero.embed(s, flair_embedding) function #146
base: master
Are you sure you want to change the base?
Conversation
missing: - docstring - tests
- add docstring - improve functionality - add dependency Co-authored-by: Maximilian Krahn <maximilian.krahn@icloud.com>
Build fails on Windows and errors on macOS, will look into it EDIT linux+macOS passing now; still working on Windows build EDIT 2 all passing now; see above for explaination |
Nice and very beautiful! 🎉 Why is What about the discussion/questions there: #140. Q: A distinction we might want to do is that in general the |
Flair tries to automatically install torch, but it fails on macOS and Windows. We could probably open a separate Issue/PR after this is merged to clean up the pipeline etc. , similar to #137 . You can e.g. see here when selecting the Windows configuration that it won't just install with
EDIT: see my answer on #140 |
If I well understood what you are saying, on a clean macOS/windows machines, What's a bit unclear is that I haven't found anything on the flair documentation that explains how |
Hmm I'm actually not sure (and also not sure how to try that out), I'll try again with some |
We have now tried out lots of stuff in #149 .
We see the following options:
What do you think? 🙃 |
We still need to "fake" on Travis CI and install flair as you were doing (and torch eventually) otherwise, unit-tests will fail
😂😂😂😂 |
- additionally set testing output to quiet as flair tests print a lot of unnecessary stuff
Great, we've now switched flair to a dev dependency and made the necessary changes in the code |
(Try to) fix macOS pipeline error.
Concerning Travis: We've now moved back to one
We believe that the |
TODO:
|
embeddings.py
hero.embed(s: TokenSeries, flair_embedding: flair.embeddings.DocumentEmbeddings) -> VectorSeries
test_embeddings.py
flair>=0.5.1.
The new function adds full support for Flair Document Embeddings. See
embeddings.py
docstrings for further explainations.EDIT:
As you can see below, we had some (many 🙃 ) travis problems. Root cause was the added dependency
flair
that introduces a lot of overhead through it's dependencies (mainlytorch
). Problems and solutions (in.travis.yml
) were:permission denied
errors when installing torch and related dependencies ➡️ Solution: install packages with--user
flag, sopip3 install --user ".[dev]" .
for macOSpip3 install --no-deps torch===1.6.0 -f https://download.pytorch.org/whl/torch_stable.html
beforepip3 install ".[dev]" .
.Overall, we had to declare different
install
steps for the different OSes (python as before, windows and macOS as described above) as you can see in the.travis.yml
file.