rom1504 / embedding-reader

Efficiently read embedding in streaming from any filesystem
MIT License
92 stars 19 forks source link

[Not yet tested] Ability to load 3 dimensions worth of embeddings #29

Closed TheoCoombes closed 7 months ago

TheoCoombes commented 2 years ago

This feature would be useful for scripts such as ClipCap that may need to load multiple patches worth of embeddings per sample, i.e. (batch, patches, dim) instead of (batch, dim).

I have not yet tested this (especially with the parquet reader, however, this is using some of the code used previously before the ClipCap rewrite. Will try it out on a numpy reader just to verify it all works okay shortly.

This also changes reader.dimension from an int to a tuple of either (dim,) or (dim1, dim2).

Is this worthy of a PR or would this be more suitable as just a modified version of the parquet_numpy_reader on ClipCap's end to avoid complication?

rom1504 commented 2 years ago

hi, yes I think that's good

however I think we need tests here and some speed benchmarks to make sure it works and it's not slowing things down

(also run make black lint to fix the lint)

TheoCoombes commented 2 years ago

I think it should generally be okay as this is merely one slight change in the regex used to load the files initially (and therefore only occurs once) - 2d arrays should be unaffected.

However, it would still most likely be important to test either way (especially with parquets where I'm not too familiar).

rom1504 commented 7 months ago

please re open if you still need this