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?

outlines/tests/generate/test_integration_vllm.py

Line 236 in 9f15e28

def test_vllm_cfg(model):

[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:

outlines/outlines/generate/api.py

Line 189 in 4f8433d

fsms = [self.fsm.copy() for _ in range(batch_size * num_samples)]

[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!