-
Notifications
You must be signed in to change notification settings - Fork 426
Migrate devsite notebooks to gemma-cookbook #276
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
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Summary of ChangesHello @bebechien, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request involves the migration of a substantial collection of educational and example notebooks related to various Gemma models and their applications. The primary goal is to centralize these resources within the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request is a significant contribution, migrating a large number of dev-site notebooks into the gemma-cookbook. The notebooks cover a wide range of topics, from basic inference to fine-tuning for various Gemma family models. While the migration is a great step, I've found several issues across the notebooks, including broken links, typos, grammatical errors, and some critical code errors that would prevent users from running the tutorials successfully. These include incorrect model identifiers, broken image URLs, and flawed code logic in examples. I've provided detailed comments and suggestions for each issue to help improve the quality and usability of these valuable resources.
| "processor = AutoTokenizer.from_pretrained(args.output_dir)\n", | ||
| "processor.save_pretrained(\"merged_model\")" |
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.
| "if model_id == \"google/gemma-3-1b-pt\":\n", | ||
| " model_class = AutoModelForCausalLM\n", | ||
| "else:\n", | ||
| " model_class = AutoModelForImageTextToText\n", |
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.
The code incorrectly selects AutoModelForImageTextToText for text-only models like google/gemma-3-4b-pt. This will cause an error. The correct class for causal language models is AutoModelForCausalLM. Please update the logic to use the correct model class.
if model_id == "google/gemma-3-1b-pt":
model_class = AutoModelForCausalLM
else:
model_class = AutoModelForCausalLM
| "id": "QITD66qJvCTO" | ||
| }, |
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.
The image URLs point to localhost. This will fail for any user running this notebook in a cloud environment like Colab or Kaggle, as the images won't be accessible. To make this tutorial runnable, please host these images publicly and update the URLs, or include them in the repository and load them from a local path.
| "model_config.dtype = \"float32\" if MACHINE_TYPE == \"cpu\" else \"float16\"\n", | ||
| "model_config.tokenizer = tokenizer_path" | ||
| ] | ||
| }, | ||
| { | ||
| "cell_type": "markdown", | ||
| "metadata": { | ||
| "id": "zSp7oLLwjKCa" |
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.
| "dog_a = read_image(\"http://localhost/images/dog-a.jpg\")\n", | ||
| "dog_b = read_image(\"http://localhost/images/dog-b.jpg\")\n", |
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.
The image URLs point to localhost. This will fail for any user running this notebook in a cloud environment like Colab or Kaggle, as the images won't be accessible. To make this tutorial runnable, please host these images publicly and update the URLs, or include them in the repository and load them from a local path.
| " <a target=\"_blank\" href=\"https://ai.google.dev/gemma/docs/core/huggingface_inference\"><img src=\"https://ai.google.dev/static/site-assets/images/docs/notebook-site-button.png\" height=\"32\" width=\"32\" />View on ai.google.dev</a>\n", | ||
| " </td>\n", | ||
| " <td>\n", | ||
| " <a target=\"_blank\" href=\"https://colab.research.google.com/github/google-gemini/gemma-cookbook/blob/main/DevSite/docs/core/huggingface_inference.ipynb\"\"><img src=\"https://www.tensorflow.org/images/colab_logo_32px.png\" />Run in Google Colab</a>\n", |
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.
There's an extra double quote at the end of the href attribute, which makes the HTML invalid and might cause rendering issues or broken links. Please remove it.
<a target="_blank" href="https://colab.research.google.com/github/google-gemini/gemma-cookbook/blob/main/DevSite/docs/core/huggingface_inference.ipynb"><img src="https://www.tensorflow.org/images/colab_logo_32px.png" />Run in Google Colab</a>
| "source": [ | ||
| "# Video analysis with Gemma\n", | ||
| "\n", | ||
| "Video data is a rich source of information that can help you accomplish tasks and understand the world around you. Using Gemma with video data can help you understand spacial relationships, interpret human interactions, and assist with situational awareness. This tutorial shows you how to get started processing video data with Gemma using Hugging Face Transformers. The Transformers Python library provides a API for accessing pre-trained generative AI models, including Gemma. For more information, see the [Transformers](https://huggingface.co/docs/transformers/en/index) documentation.\n", |
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.
I found a couple of typos and a grammatical error in this sentence. "spacial" should be "spatial", and "a API" should be "an API". Correcting these will improve the clarity of the documentation.
Video data is a rich source of information that can help you accomplish tasks and understand the world around you. Using Gemma with video data can help you understand spatial relationships, interpret human interactions, and assist with situational awareness. This tutorial shows you how to get started processing video data with Gemma using Hugging Face Transformers. The Transformers Python library provides an API for accessing pre-trained generative AI models, including Gemma. For more information, see the [Transformers](https://huggingface.co/docs/transformers/en/index) documentation.
| " <a target=\"_blank\" href=\"https://ai.google.dev/gemma/docs/capabilities/vision/video-understanding\"><img src=\"https://ai.google.dev/static/site-assets/images/docs/notebook-site-button.png\" height=\"32\" width=\"32\" />View on ai.google.dev</a>\n", | ||
| " </td>\n", | ||
| " <td>\n", | ||
| " <a target=\"_blank\" href=\"https://colab.research.google.com/github/google-gemini/gemma-cookbook/blob/main/DevSite/docs/capabilities/vision/video-understanding.ipynb\"\"><img src=\"https://www.tensorflow.org/images/colab_logo_32px.png\" />Run in Google Colab</a>\n", |
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.
There's an extra double quote at the end of the href attribute, which makes the HTML invalid and might cause rendering issues or broken links. Please remove it.
<a target="_blank" href="https://colab.research.google.com/github/google-gemini/gemma-cookbook/blob/main/DevSite/docs/capabilities/vision/video-understanding.ipynb"><img src="https://www.tensorflow.org/images/colab_logo_32px.png" />Run in Google Colab</a>
| "id": "3_lX1k54KKrx" | ||
| }, | ||
| "source": [ | ||
| "Copyright 2024 Google LLC." |
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.
| "source": [ | ||
| "Try the `MTNTDatasetBuilder` out by instantiating the custom `GriffinTokenizer` again, then applying it on the MTNT dataset, and sampling two examples:" | ||
| ] | ||
| }, | ||
| { | ||
| "cell_type": "code", | ||
| "execution_count": null, |
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.
irbg
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.
Suggest dropping the DevSite directory and merging the README note into the root README.md
I'm fine with docs/ being lowercase as it works better with the URL that will be used in Vertex/Kaggle/Colab etc
No description provided.