-
Notifications
You must be signed in to change notification settings - Fork 3
Remove LoggingLLMHandler
#508
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
Conversation
|
The issue with this PR is that, as @weave.op
@Template.define
def haiku(topic: str) -> str:
"""Generate a haiku about the topic {topic}."""
raise NotImplementedError
@weave.op
@Template.define
def poetry_compilation(topic: str) -> list[str]:
"""Generate a collection of poems surrounding {topic}, calling `haiku` on related topics to generate the elements."""
raise NotImplementedError
with handler(LiteLLMProvider()):
for poem in poetry_compilation('apple'):
print(poem)
print('') |
|
Following discussions from #509, updating documentation to instead describe adding a custom handler. |
a564ca3 to
e37b992
Compare
eb8680
left a comment
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.
Closing #493 would require deleting the current logging handler from providers.py. Also, I'm just not sure we want to add weave as a library-level dependency in effectful[llm] or include this particular code snippet in the documentation when it's not something we've tested extensively ourselves - I can believe this code runs, but it's not associated with any unit tests and there's no way for someone to tell from this notebook cell if it solves the right problem and doesn't interact poorly with other components in surprising ways. I really think we should just leave out any recommendation like this unless it's something we've tested extensively ourselves or it's something built into LiteLLM.
|
okay sounds good. I'll drop the documentation change and drop the logging handler. |
e37b992 to
77a0109
Compare
77a0109 to
e532eed
Compare
|
@eb8680 dropped llmlogginghandler. Yixiu makes a lot of use of wandb, so I'll ask him to try out the wandb handler just in MARA for now and if it's popular and gets used internally sufficiently then we can document it later. |
LoggingLLMHandler
This PR adds documentation for the fact that
weave.opis compatible with effectful and litellm.completions. In particular to getweavelogging for template calls it simply suffices to add the@weave.opdecorator to a template definition.Closes #493