-
Notifications
You must be signed in to change notification settings - Fork 41
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
Allow just state update + Parallel tracking #225
Conversation
TODO in this PR:
|
b878ecb
to
8226aca
Compare
78904f9
to
1d9b0e9
Compare
1d9b0e9
to
a56a60a
Compare
1148692
to
fa9ec33
Compare
A node can now query for application context by setting an input as __context -- this is of type ApplicationContext. This will allow forking (+ any other information we may need from inside the node). This is one of two __ variables (the other being tracer). The dependency factory is a bit in flux but the way these are constructed is not yet public-facing (we cannot arbitrarily add global injectors), so we can change it. This also fixes a bug in which dependency factories were not allowed to be optional/have defaults. This enables the user to build out a function that can be run outside the app (with a null context)..
a56a60a
to
940618d
Compare
child=PointerModel( | ||
app_id=app_id, | ||
sequence_id=None, | ||
partition_key=None, # TODO -- get partition key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what will this impact without it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the case that app IDs are non-unique it will be ambiguous
docs/concepts/recursion.rst
Outdated
This can enable the following use-cases (among many others): | ||
|
||
1. Parallel results to different LLMs for comparison | ||
2. Multi simultaneous tasks/chains for an LLM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2. Multi simultaneous tasks/chains for an LLM | |
2. Multiple simultaneous tasks/chains for an LLM |
docs/concepts/recursion.rst
Outdated
1. Parallel results to different LLMs for comparison | ||
2. Multi simultaneous tasks/chains for an LLM | ||
3. Black-boxing an action as a subset of tasks | ||
4. Running multiple queries simultaneously and getting an LLM to synthesize. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't this the same as (2) but more simply worded?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No on is different questions and the other is the same question but muliple queries
docs/concepts/serde.rst
Outdated
Field level serialization/deserialization is handled by a registration function in the state module. | ||
Fields will be first checked to see if there is a custom serializer/deserializer registered for that field, | ||
before delegating to the default serialization/deserialization mechanism. | ||
Field level serialization/deserialization is for when you want to further customize your state serialization |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this in here? bad rebase?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fixed
This one replaced #218 -- same stuff. |
Requires this PR: DAGWorks-Inc/burr#225
f2ad592
to
ad5e95c
Compare
This enables an application to specify the app that spawned it. - Adds tracking of spawning parent to child app. This enables us to track from child app the parents that spawned it - Adds pointers from parent app to child app. This does a little trickiness with the local tracker, but will allow the UI to display bidirectional links for forks/spawns. - Adds visualization for spawning parents to App list. This allows us to expand/focus on children in the app list. This also hides the partition key if it doesn't exist See #211 Adds bidirectional linking to the UI for spawning/forking apps This adds the proper UI component + fixes logging/server to log correctly when spawning.
This is potentially backwards compatible, but people should really not be doing this -- in standard python convention (and burr, specifically), these are reserved for internal/special cases.
Example + concepts doc
This was always confusing. We've decided to allow just a single output to make onboarding easier. See #53. - Updates the actions docstring to have examples just show State return - Updates the hello world example to not return results for simplicity - Updates the docs to prefer the only-state return format for actions
When we built this initially, we were validating type annotation for action functions. We now no longer do this, so this removes unecessary code.
Not doing everything cause I'm lazy and I want there to be a variation.
Adds a few more safeguards
This enables the user to do what scrapegraph is doing -- run Burr in more buried places. This is not the main approach (it should be wired in directly), but it'll work in a pinch and is very low effort.
This way we can just run tests locally
ad5e95c
to
2f9b3b8
Compare
Multi-PR (sorry, rebasing was too much of a pain to not group this. Kids, don't do this at home.)
Changes
State
instead of a tupleHow I tested this
Notes
Some sample code:
Checklist