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

Build Index from regex #125

Draft
wants to merge 22 commits into
base: main
Choose a base branch
from
Draft

Build Index from regex #125

wants to merge 22 commits into from

Conversation

torymur
Copy link
Contributor

@torymur torymur commented Dec 12, 2024

This PR drops all intermediate python and related rust structures and builds an Index from regex purely in rust. Also stabilizing python bindings for Vocabulary, Index and Guide and rust interfaces for Vocabulary and Index.

Rust

  • Build Index from regex with regex-automata
  • Change Token type from String to Vec<u8>
  • Change type of Vocabulary's eos_token_id from Option<TokenId> to TokenId
  • Stabilize Index interface
  • Stabilize Index python binding interface
  • Stabilize Vocabulary interface
  • Stabilize Vocabulary python binding interface
  • Drop all intermediary structures related to interregular workflow
  • Introduce new Guide python binding
  • Add and/or port tests
  • Add docs

Python

  • Drop all intermediary structures related to interregular workflow
  • Fix existing tests to use different interface of Guide, Index & Vocabulary
  • Adjust benchmarks to use new interface

Benchmarks

Benchmarks reveal x10+ times on average improvement for index compilation comparing to the current main branch.

On my machine to the degree, where a showcase with setting longer GIL switch interval for multithreaded case doesn't exhibit difference with one thread (because it's so fast in the first place, that there is a little sense to give it more time to execute without interruption), but it depends on machine.
image_2025-01-10_10-59-33

@torymur torymur added the enhancement New feature or request label Dec 12, 2024
@rlouf
Copy link
Member

rlouf commented Dec 13, 2024

Supersedes #38?

@torymur
Copy link
Contributor Author

torymur commented Dec 13, 2024

Yes, it will

@torymur torymur force-pushed the index-from-regex-97 branch 2 times, most recently from fa36be9 to 5b8808d Compare December 18, 2024 15:50
@torymur torymur force-pushed the index-from-regex-97 branch 3 times, most recently from 88d609b to 52edc6d Compare January 7, 2025 17:54
@torymur torymur force-pushed the index-from-regex-97 branch from 1a8831a to bf6e8a6 Compare January 9, 2025 18:12
mask: List[int] = [1 if s in allowed else 0 for s in range(1, n_tokens + 1)]
tokens = model(tokens, mask=mask)
state = fsm.get_next_state(state, tokens[-1])
allowed = guide.read_next_token(tokens[-1])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dpsimpson Could you, please, take a look at this failing test: https://github.com/dottxt-ai/outlines-core/actions/runs/12695987037/job/35389081344

Interface has changed and I updated it here somewhat accordingly, but it needs to be checked, for example I added third value to Vocabulary (instead of eos token before that) just for the sake of keeping the dimensions right, I suspect that could be incorrect 😅

But overall, since its statistical, it goes over my head to fully understand the intentions and adjust properly expected testing values or construction logic, so I would really appreciate your help here 🙏

@torymur torymur force-pushed the index-from-regex-97 branch from 5ea7e10 to 3fef1d8 Compare January 10, 2025 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants