amosjyng / langchain-visualizer

Visualization and debugging tool for LangChain workflows
MIT License
718 stars 51 forks source link

Async calls do not show up as expected #55

Closed sam-cohan closed 1 year ago

sam-cohan commented 1 year ago

Hi, thank for this excellent project. I noticed that in llms/base.py you only hijack the generate not the agenerate for the BaseLLM. Why not also hijack agenerate?

Without hijacking agenerate as well, output missing the llm call: Screenshot 2023-06-17 at 11 10 36 AM

I created a simple PR which fixes this to look somewhat more reasonable, but still the final output never shows up (not sure why) Screenshot 2023-06-17 at 11 26 12 AM

If I change all my calls to be synchronous, then everything looks much nicer with prices showing up too: Screenshot 2023-06-17 at 11 27 58 AM

Would appreciate it if you look at the PR and let me know why the prices are not working and also why the final result does not appear to show up.

Thanks again for your work.

amosjyng commented 1 year ago

Hey Sam, thanks for submitting the PR! I've added a commit to fix it. It's a bit confusing, but the actual function that needs to be overridden is VisualizationWrapper.run. Because the function you created was named _agenerate instead, it does not override anything and does not actually get called. I realized that this was a problem with the existing ChatLlmAsyncVisualizer as well, and I've fixed it there too.

I've wanted to name it something different, but if I do name it something like visualize, then every single call gets "Visualize" tacked on to the end:

run

Changing this would require delving further into ICE code, which I haven't got the energy to do just yet.

Costs should now show up for async calls. Unfortunately I was not able to reproduce the lack of return values. I've added a test to check and visualize async calls. Could you please see if running python tests/chains/langchain_how_to/test_async.py gives you return values? For me it generates this visualization:

returns

(You may need to export PYTHONPATH=$PYTHONPATH:. before running the test as a Python script)

sam-cohan commented 1 year ago

Hi @amosjyng, thank you for putting in the fix. I tested it with my previous use case and indeed, now the prices do show up (yay and thanks for that). The issue with no output showing seems to also have been resolved for now.

Screenshot 2023-06-19 at 2 35 14 PM

I suspect the missing output (which still happens sometimes is a silent failure in the API. Unfortunately all the errors are suppressed so it is hard to know what happened when things go wrong. We should find a way to propagate the errors to the ICE interface or at least to the stdout for easier debugging...

Related to the above, I also tried running the test you mentioned and it fails because I have to use an organization ID in the header and your test explicitly prohibits that. Probably should update the test to allow the OpenAI-Organization to be set.

Matchers succeeded : ['method', 'scheme', 'host', 'port', 'path', 'query', 'body']
Matchers failed :
headers - assertion failure :
{'OpenAI-Organization': '<censored>'} != {}

Anyway, that is not urgent and I think the issue has been addressed so will close this ticket now. Thanks again for the quick turnaround and keep up the good work!

amosjyng commented 1 year ago

No problem, glad to help! :)