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

ell serialization refactor / distributed 2 #362

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

Conversation

alex-dixon
Copy link
Contributor

@alex-dixon alex-dixon commented Nov 2, 2024

Rough overview:

  • Production environments may benefit from an architecture where lmp versions and invocation traces are written from multiple machines that do not share the same file system
  • Ideally, these setups need not miss out on any of ell’s features. This includes realtime updates to ell studio
  • Data stores like postgres do not offer an easy way to subscribe to realtime changes. We could have an api that works off postgres change data capture. I’ve worked with this extensively to implement materialized views from changes to tables using debezium, Kafka/redpanda, and Apache Spark. It is doable, but at the end of it this would at most support realtime for one datastore. Ideally ell and its feature set should be available with other data stores, even nosql ones. Allowing studio to wire itself to an event bus lets us “read our own writes” without getting into the particulars of realtime options per data store. This makes it much easier to integrate data stores because the realtime component is solved for all of them by default.
  • The event bus itself is pluggable in the same way as SQLite or postgres is for storage backends.
  • We establish patterns for adding new functionality via conditional import of modules and Ell’s —extras. We remove barriers to adoption and promote what is more or less single file contributions to support additional event buses or data stores. See the celery library for an example of this pattern scaling to dozens of different backends: https://github.com/celery/celery/tree/main/celery/backends
  • We continue to separate of ell studio from the core and lay groundwork for async support by introducing a serialization interface that is fully async and does not use sqlmodels as input or necessarily depend on sqlalchemy. Of all the changes this is the only one that touches the core and is probably the hardest to get right. The interfaces are written and ready for use but I have held off integrating them until we gain consensus on the code that is written here.
  • Even with serialization code added to core, ell users should notice 0 changes when updating the library except perhaps the addition of new configuration options. All changes are internal.

alex-dixon and others added 30 commits October 27, 2024 08:37
* culprit is Black formatter adds space after # comment
* it doesn't always though because if the formatter raises exception, no format
* fixed by chaning `#<BV>` => `# <BV>` but for all tags

NOTE: this might not be the right way since without rerunning all the LMPs, the old source persists in the DB, so the old #<BV> tags won't get rendered right in the React app. Could instead enforce that # <BV> becomes #<BV> even after formatting, or strip out the delimiter before formatting and add it back in after.
* OpenAI API requires there be a `api_key` field although Ollama's API doesn't require it
* Fixed `_autocommit_warning` to warn when there's no client for config.autocommit_model instead of hardcoded "gpt-4o-mini" string and updated warning message accordingly
Fix minor typos in `instructor_ex.py` docstring
For non streaming responses, Bedrock usage metadata was not being captured. Updated the metadata `dict` in line with how it was being handled for streaming responses.
Update `__getattr__` to `__getattribute__` to match the name of the real method
this does not cut core's dependency on studio/sqlmodel yet but moves in that direction
- run without storage deps when no store argument to init
- throw a helpful error when store argument provided and store deps not met
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.

7 participants