Add CFG integration to VLLM
This test failed, but it's related to the llama.cpp integration.
Fixes https://github.com/outlines-dev/outlines/issues/780.
Fixed the failing test! Could you add a simple test here ?
@rlouf I see that you added the test, I guess it's done?
https://github.com/outlines-dev/outlines/blob/9f15e28edcc6c7d623dfe12445afdfafeecd1cfb/tests/generate/test_integration_vllm.py#L236
Would you mind removing the pytest.mark.xfail decorator? The test is not run currently.
@rlouf Done.
Ok I'll update the vLLM install so we can run these tests in CI. I'll edit your PR directly so don't touch anything for now!
I get an error on the CFG test, it generates tokens that are not allowed. We need to determine if it's a problem with the CFGGuide or the integration itself. Can you reproduce this locally?
E lark.exceptions.UnexpectedCharacters: No terminal matches '.' in the current parser context, at line 1 col 12
E
E --2-2-111.9.
E ^
E Expected one of:
E * RPAR
E * STAR
E * MINUS
E * SLASH
E * PLUS
E
E Previous tokens: Token('NUMBER', '111.9')
../../../.conda/envs/outlines-dev/lib/python3.10/site-packages/lark/lexer.py:598: UnexpectedCharacters
@rlouf Using the debugger, it seems that is something related to the parser. I'm going to have a look at that.
That's what I feared
@rlouf After inspecting a little bit, generate.cfg first should be fixed since it has a bug.
The def copy(self) -> "CFGGuide" should just return self instead of CFGGuide(self.cfg_string, self.tokenizer), it loses the regex_fsm when copying.
Concerning the error, the problem comes from the built regex from interactive.accepts(). It allows some tokens that interactive.accepts() doesn't ('.' in the error above).
I could reproduce the same type of error using generate.cfg without the VLLM integration.
That's what I feared
I'm going to fix it, I'll open a draft PR when the time comes.
Thank you! I'm sorry for lack of responsiveness here, I'm really busy with other things but will have more time soon :)
No problem!
@rlouf After inspecting a little bit,
generate.cfgfirst should be fixed since it has a bug.The
def copy(self) -> "CFGGuide"should just returnselfinstead ofCFGGuide(self.cfg_string, self.tokenizer), it loses theregex_fsmwhen copying.Concerning the error, the problem comes from the built regex from
interactive.accepts(). It allows some tokens thatinteractive.accepts()doesn't ('.'in the error above).I could reproduce the same type of error using
generate.cfgwithout the VLLM integration.
I have partially debugged the bug in generate.cfg w/ this PR: https://github.com/outlines-dev/outlines/pull/865
@miftahmoha @rlouf
It's not a good idea to return self in def copy cuz then we'd end up using the same fsm for all prompts in the batch. See: https://github.com/outlines-dev/outlines/blob/4f8433d8d6633b0780c3a6c27981f9adffbe49f5/outlines/generate/api.py#L189
IMO it's better to add another method, say def deep_copy, which returns CFGGuide(self.cfg_string, self.tokenizer, self.refex_fsm, ...) and use that instead in reorder_fsms (and just in reorder_fsms for now)
Alternatively, we can somehow make CFGGuide pickle-able and just use copy.deepcopy in reorder_fsms
Concerning the error, the problem comes from the built regex from interactive.accepts(). It allows some tokens that interactive.accepts() doesn't ('.' in the error above).
Hi @miftahmoha , I didnt get you. Could you please expand more on above? Thanks!
@ekagra-ranjan Sorry for the late answer, there are two mechanisms that are in the works here.
First, the one that gives the next possible tokens following the CFG (tokens in the context of a lexer in compilers) which is interactive.accepts(). Second, one that takes those tokens and builds a regex out of them for guided generation.
There is a problem in the second mechanism.