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

Stop sequences in fireworks, plus notebook updates #11136

Merged
merged 3 commits into from Sep 27, 2023

Conversation

tjaffri
Copy link
Contributor

@tjaffri tjaffri commented Sep 27, 2023

The new Fireworks and FireworksChat implementations are awesome! Added in this PR #11117 thank you @ZixinYang

However, I think stop words were not plumbed correctly. I've made some simple changes to do that, and also updated the notebook to be a bit clearer with what's needed to use both new models.

Tagging model maintainer @baskaryan

@vercel
Copy link

vercel bot commented Sep 27, 2023

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

Name Status Preview Comments Updated (UTC)
langchain ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 27, 2023 9:55pm

@dosubot dosubot bot added Ɑ: models Related to LLMs or chat model modules 🤖:improvement Medium size change to existing code to handle new use-cases labels Sep 27, 2023
@tjaffri tjaffri marked this pull request as ready for review September 27, 2023 20:43
@baskaryan
Copy link
Collaborator

lgtm, would be good to have @ZixinYang take a look, too

@ZixinYang
Copy link
Contributor

ZixinYang commented Sep 27, 2023

Looks good to me! Thanks @tjaffri for catching this issue.

@ZixinYang
Copy link
Contributor

ZixinYang commented Sep 27, 2023

btw, @tjaffri could you also help to change the title of the notebook? https://python.langchain.com/docs/integrations/chat/fireworks
It should be Fireworks rather than ChatFireworks so the name on the sidebar is expected to change accordingly. Thank you!!!

@tjaffri
Copy link
Contributor Author

tjaffri commented Sep 27, 2023

btw, @tjaffri could you also help to change the title of the notebook? https://python.langchain.com/docs/integrations/chat/fireworks So that it can be Fireworks, not ChatFireworks. Thanks a lot!!!

Good catch... I didn't even notice there was this other notebook! So actually its title is correct... there are TWO notebooks:

  1. Fireworks: https://python.langchain.com/docs/integrations/llms/fireworks
  2. ChatFireworks: https://python.langchain.com/docs/integrations/chat/fireworks

I actually only updated the first one and put my chat related notebook updates in that one... I will just move that section from the second one to the first one, which should do the trick...

@ZixinYang
Copy link
Contributor

btw, @tjaffri could you also help to change the title of the notebook? https://python.langchain.com/docs/integrations/chat/fireworks So that it can be Fireworks, not ChatFireworks. Thanks a lot!!!

Good catch... I didn't even notice there was this other notebook! So actually its title is correct... there are TWO notebooks:

  1. Fireworks: https://python.langchain.com/docs/integrations/llms/fireworks
  2. ChatFireworks: https://python.langchain.com/docs/integrations/chat/fireworks

I actually only updated the first one and put my chat related notebook updates in that one... I will just move that section from the second one to the first one, which should do the trick...

Sounds good. For the notebook under chat folder. I would like to change the title to Fireworks as well in order to align with other APIs. Other APIs use chat model in the notebook but keeps their name in title. I feel in this way, it is easier to search our Fireworks notebook from side bar.

@tjaffri
Copy link
Contributor Author

tjaffri commented Sep 27, 2023

Sounds good. For the notebook under chat folder. I would like to change the title to Fireworks as well in order to align with other APIs. Other APIs use chat model in the notebook but keeps their name in title. I feel in this way, it is easier to search our Fireworks notebook from side bar.

done... please let me know if it looks ok to you

@ZixinYang
Copy link
Contributor

ZixinYang commented Sep 27, 2023

@tjaffri Looks good! Thanks again for the change. The instructions in the notebook have also been greatly improved. @baskaryan could you help to merge the changes? Thanks!

@baskaryan baskaryan merged commit b7e9db5 into langchain-ai:master Sep 27, 2023
32 checks passed
@tjaffri tjaffri deleted the tjaffri/fireworks branch September 27, 2023 23:12
3coins pushed a commit to 3coins/langchain that referenced this pull request Sep 28, 2023
The new Fireworks and FireworksChat implementations are awesome! Added
in this PR langchain-ai#11117 thank
you @ZixinYang

However, I think stop words were not plumbed correctly. I've made some
simple changes to do that, and also updated the notebook to be a bit
clearer with what's needed to use both new models.


---------

Co-authored-by: Taqi Jaffri <tjaffri@docugami.com>
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 Ɑ: models Related to LLMs or chat model modules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants