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

SentenceEmbeddingsModel is not Sync #389

Closed
valyagolev opened this issue Jun 2, 2023 · 2 comments
Closed

SentenceEmbeddingsModel is not Sync #389

valyagolev opened this issue Jun 2, 2023 · 2 comments

Comments

@valyagolev
Copy link

Is there any fundamental reason SentenceEmbeddingsModel would not be Sync? I'm a bit confused about this.

If there's no such reason, I will try to see what's the reason it's not Sync and fix it when I find time.

@guillaume-be
Copy link
Owner

Hello,
As far as I know the tch::Tensor, thoroughly used in this crate, is Send but not Sync, and therefore all models and pipelines are also not Sync. They should however all be Send, this is enforced by unit test checks in the crate.

@valyagolev
Copy link
Author

OK thank you -

I found this explanation here: LaurentMazare/tch-rs#12 (comment) which mentions that ...

some storage may be shared between tensors and lead to unsound behavior.

In practice this is not much of an issue, but doing more operations across thread is likely to trigger more race conditions

probably having Send is already an issue here

which I don't 100% understand (is "Send" then also unsound? what are the conditions for this happening, is this even possible in case of just using downloaded embeddings?). But yeah, I guess I see why one wouldn't want to open this can of worms. Thanks, I'll try to invent some kind of a wrapper for my use

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

No branches or pull requests

2 participants