Skip to content
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

Reuse jinja environment for a prompt #1162

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jantrienes
Copy link

@jantrienes jantrienes commented Sep 19, 2024

I really like the @outlines.prompt decorator. However, I noticed it is slow when formatting a large number of examples. The main inefficiency is creating the Jinja environment for every call of render. This PR makes a minor change to re-use the same Jinja environment per prompt.

Note: This makes the render function obsolete. However, I kept it for backwards compatibility. Please advise if this should be further refactored.

Benchmark

Small benchmark to test it:

import timeit
import outlines

@outlines.prompt
def test_tpl(variable):
    """{{variable}} test"""

print('Formatted template:', test_tpl(variable='this is a'))

def test_render():
    test_tpl(variable='this is a')

execution_time = timeit.timeit(test_render, number=1000)
print(f"Execution time for 1000 runs: {execution_time} seconds")

With main:

Formatted template: this is a test
Execution time for 1000 runs: 0.11741420900216326 seconds

With this PR:

Formatted template: this is a test
Execution time for 1000 runs: 0.004638582991901785 seconds

@cpfiffer
Copy link
Contributor

Big fan of this PR, free speed = dope.

Note: This makes the render function obsolete. However, I kept it for backwards compatibility. Please advise if this should be further refactored.

Could you add a comment or something to the render method so people are aware it's a no-op?

I'm also curious if anyone's actually using it at the lowered level. I suspect this group of people is relatively small, in which case just removing the method is probably fine with an appropriate deprecation warning.

We might also like to have tests using 1-2 different variables as well to test that this still works in both cases.

@rlouf
Copy link
Member

rlouf commented Sep 19, 2024

Great PR! We are moving this logic to our new prompts library, and it would be awesome if you could submit a PR there as well!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants