-
Notifications
You must be signed in to change notification settings - Fork 215
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 combiner packing for tft.vocabulary #259
Comments
Due to implementation differences (i.e. vocabulary is not combiner based) this is a much more complex task to apply packing to vocabulary as opposed to approximate_vocabulary unfortunately, without causing regressions that is. @cyc, could you please describe what it is you're hoping to accomlish for your pipeline through vocabulary analyzer packing? |
@zoyahav the application is similar to what is described in #260. The issue is essentially the same as the previous issue, #180. It seems like having large numbers of unpacked analyzers has some negative effects on performance. I noticed that the implementation of the analyzer seemed to have changed between Feel free to close this as "wontfix" if this isn't on the roadmap. I was more just inquiring about whether this has been considered or not. |
The main reason why The reason why we can't use this logic in I looked into some other ways of packing the vocabulary computation (different from what is done for Also worth noting that if Sorry for a late response, just came across this. |
I'm sure this is likely on the roadmap already, but it would be very beneficial for data transformations that require many simultaneous vocabulary computations to support combiner packing the same way it is supported for tft.experimental.approximate_vocabulary.
This may be a duplicate of #180 (comment) but I think it is worth re-raising given that this has been implemented for approximate_vocabulary.
The text was updated successfully, but these errors were encountered: