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

support returning run info for llms, chat models and chains #5666

Merged
merged 4 commits into from
Jun 6, 2023

Conversation

agola11
Copy link
Collaborator

@agola11 agola11 commented Jun 3, 2023

returning the run id is important for accessing the run later on

@@ -108,6 +108,8 @@ def __call__(
inputs: Union[Dict[str, Any], Any],
return_only_outputs: bool = False,
callbacks: Callbacks = None,
*,
include_run_info: bool = False,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why make this optional?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mainly for better bw-compat, we have a bunch of existing tests that check the returned dict from __call__ and expect it not to have a __run field. Other people might have tests like that too.

@@ -93,6 +94,8 @@ def generate(
generations = [res.generations for res in results]
output = LLMResult(generations=generations, llm_output=llm_output)
run_manager.on_llm_end(output)
if run_manager:
output.run = RunInfo(run_id=run_manager.run_id)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be RUN_KEY

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfortunately you cannot set underscore keys in pydantic objects, i.e. LLMResult

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow these python libraries are something else

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could set the alias then serialize with by_alias=True though maybe that's even more complicated

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah... seems more complex than it needs to imo

@agola11 agola11 merged commit b177a29 into master Jun 6, 2023
12 checks passed
@agola11 agola11 deleted the ankush/return-id branch June 6, 2023 17:07
Undertone0809 pushed a commit to Undertone0809/langchain that referenced this pull request Jun 19, 2023
…n-ai#5666)

returning the run id is important for accessing the run later on
This was referenced Jun 25, 2023
kacperlukawski pushed a commit to kacperlukawski/langchain that referenced this pull request Jun 29, 2023
…n-ai#5666)

returning the run id is important for accessing the run later on
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.

None yet

3 participants