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

fnllm Updates #326

Merged
merged 17 commits into from
Dec 20, 2024
Merged

fnllm Updates #326

merged 17 commits into from
Dec 20, 2024

Conversation

darthtrevino
Copy link
Member

@darthtrevino darthtrevino commented Dec 19, 2024

  • Add type-checking blocks to most files to improve load time
  • Add audit fields 'created', and 'updated' in cache entries to support cache clearing with TTLs
  • Only emit usage metrics on cache misses to improve model consumption accuracy.

fixes #325

@andresmor-ms
Copy link
Contributor

Regarding load times... Have you considered looking into the init files and separate by modules? for example, leave the Limiters stuff in fnllm.limiters, and exposing less things into the fnllm package?

@darthtrevino
Copy link
Member Author

That's not a bad idea, but it's probably worth a longer discussion on how to break that apart. We could just expose top-level types from fnlm, or we could expose nothing; let's address breaking down the barrel-files in a future PR

@AlonsoGuevara
Copy link

Just to add on top of Andres' comment. The benefit from type check imports isn't that significant, surely improves time (and Ruff checks them) but the bang for the buck is minimizing how much you're exposing on your top level modules. If everything rolls up to top level, you're basically loading everything and not benefiting from how the Python interpreter lazy loads this stuff.

We did this on graphRAG and load time went from 2 minutes to seconds.

python/fnllm/fnllm/caching/file.py Outdated Show resolved Hide resolved
python/fnllm/fnllm/openai/llm/chat_text.py Show resolved Hide resolved
python/fnllm/fnllm/services/cache_interactor.py Outdated Show resolved Hide resolved
@darthtrevino darthtrevino merged commit 74bf8ec into main Dec 20, 2024
17 checks passed
@darthtrevino darthtrevino deleted the task/fnllm_type_checking_blocks branch December 20, 2024 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fnllm - Add audit timestamp fields for cache cleaning tooling
4 participants