huggingface / huggingface-llama-recipes

531 stars 59 forks source link

Issue #44 Add Gradio Demos for Llama Models - Initial Contribution #67

Closed Zhreyu closed 3 weeks ago

Zhreyu commented 1 month ago

Hello,

@srinjoydutta03 and I are excited to contribute to the LLaMA Recipes Repository with our initial groundwork for #44. This PR includes five Gradio demos that showcase few of the Llama model functionalities, along with a README file to guide users through the demos.

We would greatly appreciate your feedback on the notebooks and their structure. Once finalized, we also plan to add contribution guidelines to make it easier for others to add new notebooks to the folder.

Looking forward to your suggestions and feedback!

Zhreyu commented 1 month ago

Hey @ariG23498, whenever you get a chance, could you please review our pull request? Thank you!

srinjoydutta03 commented 1 month ago

Thanks for the review. We will start working on the changes right away.

Zhreyu commented 1 month ago

Hello @ariG23498, we have made the changes you requested, please do review whenever you can. Thank you.

ariG23498 commented 1 month ago

We are really close on zeroing down this PR. I tried running the notebook, but it errored out. I think it has to do with the load_model not being defined.

After that is figured out, I can review it again. 🤗

Zhreyu commented 1 month ago

Thanks for reviewing, could you please share us the exact error? we have defined a load_model function in the notebook and it was working properly.

ariG23498 commented 1 month ago

Ah! My bad, I see the function.

Some suggestions:

  1. As we are only interested in a single notebook, let's remove the README from the folder
  2. Add the gradio demo notebook in the main README (.github/README.md)
  3. The top of the notebook feels very heavy on the login and access token, we can replace that bit with a small sentence of two.
  4. I think having three system prompts is an overkill, ideally a user could say "summarize the following ..." and the model would be able to summarize the text provided. I think removing the switching logic for system prompts would be more clean

Hope this helps!

srinjoydutta03 commented 1 month ago

Thanks for the suggestions, we have updated the cell descriptions and included cell outputs, along with it we have added the chatbot screenshot in the notebook. We have also refactored the generate_chat function for direct gradio compatibility (chat messages format in place of tuples format previously). Please review the changes whenever you can @Vaibhavs10.

Zhreyu commented 3 weeks ago

Thanks for reviewing, We have added the python script.