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

core, standard tests, partner packages: add test for model params #21677

Merged
merged 34 commits into from
May 17, 2024

Conversation

ccurme
Copy link
Collaborator

@ccurme ccurme commented May 14, 2024

  1. Adds .get_ls_params to BaseChatModel which returns
class LangSmithParams(TypedDict, total=False):
    ls_provider: str
    ls_model_name: str
    ls_model_type: Literal["chat"]
    ls_temperature: Optional[float]
    ls_max_tokens: Optional[int]
    ls_stop: Optional[List[str]]

by default it will only return

{ls_model_type="chat", ls_stop=stop}
  1. Add these params to inheritable metadata in CallbackManager.configure

  2. Implement .get_ls_params and populate all params for Anthropic + all subclasses of BaseChatOpenAI

Sample trace: https://smith.langchain.com/public/d2962673-4c83-47c7-b51e-61d07aaffb1b/r

OpenAI:
Screenshot 2024-05-17 at 10 03 35 AM

Anthropic:
Screenshot 2024-05-17 at 10 06 07 AM

Mistral (and all others for which params are not yet populated):
Screenshot 2024-05-17 at 10 08 43 AM

Copy link

vercel bot commented May 14, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Visit Preview May 16, 2024 11:24pm

@efriis efriis added the partner label May 15, 2024
@efriis efriis self-assigned this May 15, 2024
@ccurme ccurme changed the title standard tests: add test for model params core, standard tests, openai: add test for model params May 15, 2024
@ccurme ccurme marked this pull request as ready for review May 15, 2024 14:15
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. 🔌: openai Primarily related to OpenAI integrations 🤖:improvement Medium size change to existing code to handle new use-cases labels May 15, 2024
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels May 15, 2024
@ccurme
Copy link
Collaborator Author

ccurme commented May 15, 2024

@ccurme ccurme changed the title core, standard tests, openai: add test for model params core, standard tests, partner packages: add test for model params May 15, 2024
@ccurme ccurme requested a review from baskaryan May 16, 2024 17:15
@@ -60,6 +63,15 @@
from langchain_core.tools import BaseTool


class LangSmithParams(TypedDict, total=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 using both total=False and 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.

this is kind of an abuse of TypedDict because I expect fields to be missing.

here I include all parameters that we are standardizing (including those that can be None). this is mostly just to define them somewhere.

regarding total=False: the implementation of _get_ls_params on BaseChatModel is almost the same as _get_invocation_params() (it just adds ls_model_type="chat"). so there should be minimal changes in tracing to models on which we haven't implemented _get_ls_params yet. but providers generally don't have the ls_ params yet.

@ccurme ccurme requested a review from nfcampos May 16, 2024 17:15
@ccurme
Copy link
Collaborator Author

ccurme commented May 16, 2024

**kwargs: Any,
) -> LangSmithParams:
"""Get standard params for tracing."""
return LangSmithParams(ls_model_type="chat")
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we want this default implementation, and why does it ignore stop words?

@ccurme ccurme merged commit 181dfef into master May 17, 2024
221 of 224 checks passed
@ccurme ccurme deleted the cc/add_test_for_model_params branch May 17, 2024 17:51
jeromechoo pushed a commit to jeromechoo/langchain that referenced this pull request May 20, 2024
…ngchain-ai#21677)

1. Adds `.get_ls_params` to BaseChatModel which returns
```python
class LangSmithParams(TypedDict, total=False):
    ls_provider: str
    ls_model_name: str
    ls_model_type: Literal["chat"]
    ls_temperature: Optional[float]
    ls_max_tokens: Optional[int]
    ls_stop: Optional[List[str]]
```
by default it will only return
```python
{ls_model_type="chat", ls_stop=stop}
```

2. Add these params to inheritable metadata in
`CallbackManager.configure`

3. Implement `.get_ls_params` and populate all params for Anthropic +
all subclasses of BaseChatOpenAI

Sample trace:
https://smith.langchain.com/public/d2962673-4c83-47c7-b51e-61d07aaffb1b/r

**OpenAI**:
<img width="984" alt="Screenshot 2024-05-17 at 10 03 35 AM"
src="https://github.com/langchain-ai/langchain/assets/26529506/2ef41f74-a9df-4e0e-905d-da74fa82a910">

**Anthropic**:
<img width="978" alt="Screenshot 2024-05-17 at 10 06 07 AM"
src="https://github.com/langchain-ai/langchain/assets/26529506/39701c9f-7da5-4f1a-ab14-84e9169d63e7">

**Mistral** (and all others for which params are not yet populated):
<img width="977" alt="Screenshot 2024-05-17 at 10 08 43 AM"
src="https://github.com/langchain-ai/langchain/assets/26529506/37d7d894-fec2-4300-986f-49a5f0191b03">
JuHyung-Son pushed a commit to JuHyung-Son/langchain that referenced this pull request May 23, 2024
…ngchain-ai#21677)

1. Adds `.get_ls_params` to BaseChatModel which returns
```python
class LangSmithParams(TypedDict, total=False):
    ls_provider: str
    ls_model_name: str
    ls_model_type: Literal["chat"]
    ls_temperature: Optional[float]
    ls_max_tokens: Optional[int]
    ls_stop: Optional[List[str]]
```
by default it will only return
```python
{ls_model_type="chat", ls_stop=stop}
```

2. Add these params to inheritable metadata in
`CallbackManager.configure`

3. Implement `.get_ls_params` and populate all params for Anthropic +
all subclasses of BaseChatOpenAI

Sample trace:
https://smith.langchain.com/public/d2962673-4c83-47c7-b51e-61d07aaffb1b/r

**OpenAI**:
<img width="984" alt="Screenshot 2024-05-17 at 10 03 35 AM"
src="https://github.com/langchain-ai/langchain/assets/26529506/2ef41f74-a9df-4e0e-905d-da74fa82a910">

**Anthropic**:
<img width="978" alt="Screenshot 2024-05-17 at 10 06 07 AM"
src="https://github.com/langchain-ai/langchain/assets/26529506/39701c9f-7da5-4f1a-ab14-84e9169d63e7">

**Mistral** (and all others for which params are not yet populated):
<img width="977" alt="Screenshot 2024-05-17 at 10 08 43 AM"
src="https://github.com/langchain-ai/langchain/assets/26529506/37d7d894-fec2-4300-986f-49a5f0191b03">
hinthornw pushed a commit that referenced this pull request Jun 20, 2024
…1677)

1. Adds `.get_ls_params` to BaseChatModel which returns
```python
class LangSmithParams(TypedDict, total=False):
    ls_provider: str
    ls_model_name: str
    ls_model_type: Literal["chat"]
    ls_temperature: Optional[float]
    ls_max_tokens: Optional[int]
    ls_stop: Optional[List[str]]
```
by default it will only return
```python
{ls_model_type="chat", ls_stop=stop}
```

2. Add these params to inheritable metadata in
`CallbackManager.configure`

3. Implement `.get_ls_params` and populate all params for Anthropic +
all subclasses of BaseChatOpenAI

Sample trace:
https://smith.langchain.com/public/d2962673-4c83-47c7-b51e-61d07aaffb1b/r

**OpenAI**:
<img width="984" alt="Screenshot 2024-05-17 at 10 03 35 AM"
src="https://github.com/langchain-ai/langchain/assets/26529506/2ef41f74-a9df-4e0e-905d-da74fa82a910">

**Anthropic**:
<img width="978" alt="Screenshot 2024-05-17 at 10 06 07 AM"
src="https://github.com/langchain-ai/langchain/assets/26529506/39701c9f-7da5-4f1a-ab14-84e9169d63e7">

**Mistral** (and all others for which params are not yet populated):
<img width="977" alt="Screenshot 2024-05-17 at 10 08 43 AM"
src="https://github.com/langchain-ai/langchain/assets/26529506/37d7d894-fec2-4300-986f-49a5f0191b03">
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖:improvement Medium size change to existing code to handle new use-cases 🔌: openai Primarily related to OpenAI integrations partner size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants