-
Notifications
You must be signed in to change notification settings - Fork 560
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
Endless generation bug popped up during migration to Guide
in vLLM integration
#856
Comments
Thanks for the issue and your work integrating Outlines new Guide interface into vLLM, @br3no! Could you please check this PR, I believe I've successfully reproduced your issue and resolved it: #884 Your test case passes with the PR on my end. You can prototype the fix with Please feel free to ping me if you run into any issues with this PR, or with integrations between Outlines and vLLM. |
@lapp0 Your PR is very similar to mine: #874. The only difference being, that in my PR the I believe they are functionally equivalent. I don't really mind which one gets merged; the important thing is getting this fix! |
And probably we should create a new issue for this: #874 (comment) Right? |
Oops, sorry I missed your PR. Yes they're functionallity identical. And I agree, this calls for a new issue. Does this seem reasonable? #885 |
Yes! |
Fixes #856 The code this PR removes introduces an artificial and erroneous loop transition in every final state that is always traversed, regardless of the generation. The comment doesn't make sense in my opinion, as the `if` above just handles exactly this case. Removing this piece of code fixes the bug that surfaced in the upgrade of outlines in the vLLM integration.
Describe the issue as clearly as possible:
While updating the vLLM integration with outlines (cf. vllm-project/vllm#4109) to move to the new
Guide
API a test started failing. The symptom is that a regular expression that matches IPv4 addresses does not stop generating.The original regular expression was:
r"((25[0-5]|(2[0-4]|1\d|[1-9]|)\d)\.){3}(25[0-5]|(2[0-4]|1\d|[1-9]|)\d)"
The issue can still be observed with the much simpler expression:
r"((25|(2|1\d|\d))\.){3}(25|(2|1\d|\d))"
The issue was discussed in a call with @rlouf.
Things we tried, that didn't change the result:
RegexGuide
code and remove thebyte_fsm
replacing it withregex_pattern.to_fsm()
Steps/code to reproduce the bug:
Expected result:
Error message:
No response
Outlines/Python version information:
Version information
Context for the issue:
No response
The text was updated successfully, but these errors were encountered: