Add CFG integration to VLLM

[miftahmoha]

This test failed, but it's related to the llama.cpp integration.

Fixes #780.

[rlouf]

Fixed the failing test! Could you add a simple test here ?

[miftahmoha]

@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

[rlouf]

Would you mind removing the pytest.mark.xfail decorator? The test is not run currently.

[miftahmoha]

@rlouf Done.

[rlouf]

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!

[rlouf]

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

[miftahmoha]

@rlouf Using the debugger, it seems that is something related to the parser. I'm going to have a look at that.

[rlouf]

That's what I feared

[miftahmoha]

@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.

[miftahmoha]

That's what I feared

I'm going to fix it, I'll open a draft PR when the time comes.

[rlouf]

Thank you! I'm sorry for lack of responsiveness here, I'm really busy with other things but will have more time soon :)

[miftahmoha]

No problem!

[leloykun]

@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.

I have partially debugged the bug in generate.cfg w/ this PR: #865

[leloykun]

@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

[leloykun]

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

[ekagra-ranjan]

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!

[miftahmoha]

@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.