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

Fix issue with use_cache #243

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix issue with use_cache #243

wants to merge 3 commits into from

Conversation

ed-fish
Copy link
Contributor

@ed-fish ed-fish commented Oct 2, 2021

Fixes issue where vector list cannot be cast to np array when loaded sequentially from cache.

Adds support for using both 2d and 1d cached vectors by expanding 1d vectors and stacking to ensure the correct shape.

@duhaime
Copy link
Member

duhaime commented Oct 2, 2021

That's interesting @ed-fish were you getting 2d arrays written to disk before? Everything should be 1d in the pipeline...

If you can say more about what led you to want to make this change, that would be helpful!

@ed-fish
Copy link
Contributor Author

ed-fish commented Oct 2, 2021

Oh yes you are correct! On second look I realised I created this error by using a dir with a mix of 1 and 2 dimensional vectors. Perhaps it is better addressed using an assert to ensure that if you use outputs from another model that they are 1d. Please feel free to reject the PR.

@duhaime
Copy link
Member

duhaime commented Oct 2, 2021

@ed-fish thanks for your follow up. An assertion that prints a useful message upon failure (e.g. Image vectors should be 1D but the image vectors found are 2D) would be a welcome PR instead if you like!

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 this pull request may close these issues.

None yet

2 participants