-
-
Notifications
You must be signed in to change notification settings - Fork 325
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
Add nvidia provider #579
Add nvidia provider #579
Conversation
Thanks for submitting your first pull request! You are awesome! 🤗 |
for more information, see https://pre-commit.ci
@stevie-35 Thank you so much for opening this pull request! Our team will take a look at it as soon as we can. The changelog is generated by our release workflows, so you don't have to modify it. Our sample notebooks like |
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.
Thank you so much for opening this request! I signed up for an API key and I was able to make a request against a couple of different models. I was a little confused about the sign-up process: I generated a key for the Llama 13B model in the NVIDIA catalog, but I was able to make requests against other models with both the magic commands and the chat UI. Is this intended behavior? If so, it might be good to note this in the docs.
Co-authored-by: Jason Weill <93281816+JasonWeill@users.noreply.github.com>
@JasonWeill Thanks for reviewing! RE: "I was able to make requests against other models with both the magic commands and the chat UI. Is this intended behavior? If so, it might be good to note this in the docs." Yes, this seems to be the expected behavior - I appreciate your note here and I will happily bring it to our internal teams as feedback! Perhaps this flow will be changed in the future, and if so, we can make a new PR to update these docs. |
I added a note about the API key behavior in the above commit. |
Hi @stevie-35 ! Thanks for your patience — I was out of office earlier this week. With an NVIDIA API key set, I get the following error when I use an NVIDIA embedding model:
I don't see errors when using an OpenAI model with a valid API key, so this might be isolated to the NVIDIA key. I do see an error with the Cohere embedding model, but this is likely unrelated to your change. |
Hey @JasonWeill I'm having trouble debugging this because of another issue, so I'm going to pull out the embedding model part and make a new PR for that later so we can get this merged. (I'm seeing a different issue where the Jupyternaut application is actually sending a copy of the api key to the LLM whenever I write a message, so I'm not able to recreate your error - thought I might share since I'm not sure what this behavior is.) Will do this now and then hopefully should be ready! |
@stevie-35 Thanks for revising this review! I have been trying the different models that your change adds. With
This doesn't occur with any of the other models in the |
Hi @JasonWeill, I just ran the same (with the single API key I have been using throughout developent) and am seeing appropriate output: Perhaps the issue has been resolved. Would you like to try again? I can report this error to our engineering teams to prevent future problems like this, but those should be solved outside this repo. Let me know if you are ok with still including the model. Thanks for testing them out and continuing to work on this! |
I still see a "deadline exceeded" error on the Mixtral model, but since this seems to be a problem on the remote end, that shouldn't block this PR. @stevie-35 I see one incomplete todo on the original description. Did you get clarity about whether to use |
@stevie-35 Any update about the |
@JasonWeill we are good to go with the current naming convention. Thank you for checking on that and for your attention on this. |
@stevie-35 Looks like I can't rebase this branch due to conflicts. If you can rebase your branch against |
@stevie-35 I can help with the rebase & merge conflict resolution if you need a hand, just LMK! |
All good, thanks for the offer @dlqqq! Just intermittently busy but hopefully we are good to go after the checks pass, @JasonWeill. I will try to be responsive over the next couple hours in case anything changes. |
@stevie-35 I saw you did a merge commit, but I'm still seeing an out-of-date message. Can you try running |
ok @JasonWeill I think I took care of it, hopefully.. |
@stevie-35 Thank you so much for your contribution! |
@meeseeksdev please backport to 1.x |
Co-authored-by: Alex Stephens <146462356+stevie-35@users.noreply.github.com>
* Add NVIDIA chat and embeddings providers * Fix typo * Some NVIDIA provider documentation * Update docs and naming for NVIDIA * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Update docs/source/users/index.md Co-authored-by: Jason Weill <93281816+JasonWeill@users.noreply.github.com> * Update index.md - add note about api key access * Remove nvidia embeddings model * Remove nvidia embedding provider * Remove nvidia embeddings provider * Mentions conda install instructions in docs * Uninstallation with conda * Makes NVIDIA package dependency optional, updates docs * Revert "Uninstallation with conda" This reverts commit 8082fa4. * Revert "Mentions conda install instructions in docs" This reverts commit 348ae2e. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Jason Weill <93281816+JasonWeill@users.noreply.github.com> Co-authored-by: Jason Weill <jweill@amazon.com>
* Add NVIDIA chat and embeddings providers * Fix typo * Some NVIDIA provider documentation * Update docs and naming for NVIDIA * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Update docs/source/users/index.md Co-authored-by: Jason Weill <93281816+JasonWeill@users.noreply.github.com> * Update index.md - add note about api key access * Remove nvidia embeddings model * Remove nvidia embedding provider * Remove nvidia embeddings provider * Mentions conda install instructions in docs * Uninstallation with conda * Makes NVIDIA package dependency optional, updates docs * Revert "Uninstallation with conda" This reverts commit 8082fa4. * Revert "Mentions conda install instructions in docs" This reverts commit 348ae2e. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Jason Weill <93281816+JasonWeill@users.noreply.github.com> Co-authored-by: Jason Weill <jweill@amazon.com>
This PR closes #568.
Notes/questions for reviewers:
nvidia
shows up in the%ai list
and other outputs?Notes for NVIDIAns
nvidia-chat
vs usingnvidia
?) or tag someone else who can?Thanks!