Add telephone examples, conversation RAG, and halt conditions #12
Conversation
skrawcz
force-pushed
the
add_telephone
branch
2 times, most recently
from
a4f970c to
0b7bc6f
Compare
| if __name__ == "__main__": | ||
| import random | ||
|
|
||
| coin_flip = random.choice([True, False]) |
There was a problem hiding this comment.
What's happening here? Why are we flipping a coin then choosing (a) which one to run and (b) which way to run it?
There was a problem hiding this comment.
Ok, so:
- Have both as options, toggled by a CLI parameter or env variable. Or just have one highlighted out.
- Don't show both ways of running it, just use
.run(have a link to the docs in which it shows how to do it, this is laid out very cleanly) - Add more comments
| return result, state.update(**updates).append(image_caption_history=current_caption) | ||
|
|
||
|
|
||
| @action(reads=[], writes=[]) |
There was a problem hiding this comment.
This should be Result or something -- writing an empty "placeholder" is an anti-pattern. Result is meant to have a state that has a final result (E.G. the location + caption history).
| ApplicationBuilder() | ||
| .with_state( | ||
| current_image_location="telephone_graph.png", | ||
| current_image_caption="", |
There was a problem hiding this comment.
None is better for initial state of current_image_caption
| generate_images = dataflows.import_module("generate_images", "elijahbenizzy") | ||
|
|
||
|
|
||
| caption_image_driver = driver.Builder().with_config({}).with_modules(caption_images).build() |
There was a problem hiding this comment.
Hmm we should really have a hub action that takes in ("caption_images", "elijahbenizzy") and delegates to Hamilton action...
| if __name__ == "__main__": | ||
| import random | ||
|
|
||
| coin_flip = random.choice([True, False]) |
There was a problem hiding this comment.
Ok, so:
- Have both as options, toggled by a CLI parameter or env variable. Or just have one highlighted out.
- Don't show both ways of running it, just use
.run(have a link to the docs in which it shows how to do it, this is laid out very cleanly) - Add more comments
the variables were round the wrong way.
This uses the dataflows from the hub and then shows two ways to integrate Hamilton. One via the Hamilton integration, the other via the function API. Also adds tracking so that we can get some nice logs of what's going on!
This one is basic and uses the off-the-shelf Hamilton dataflow for conversational RAG. You need to supply the knowledge for it.
So decisions: 1. we want something to stop before, and then stop after. hence halt_*. 2. step() will always result in running the action. 3. We need to run the first thing, so we don't get stuck in a never ending loop -- this means if you know it needs inputs, you need to pass inputs in. This updates docs + fixes examples to demonstrate the new syntax.
Before we were repeating a lot, and arun did the wrong thing. This ensures everything is uniform, and cleans up a lot of really big if/else conditions to be shared utility functions. This also clarifies documentation.
elijahbenizzy
force-pushed
the
add_telephone
branch
from
140e8bb to
5fa877d
Compare
elijahbenizzy
changed the title
There was a problem hiding this comment.
@skrawcz I'm approving this but I think we need to touch up the examples cause they're too complex and have too many weird options. Better to show one way we like more than two ways. Let's do that asynchronously though, because this is coupled to core features we want to release.
This PR is a behemoth -- we have:
That were merged in from #25