-
Notifications
You must be signed in to change notification settings - Fork 228
feat: WeatherNext2 Initial Updates #4417
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
Summary of ChangesHello @dpanigra, 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 integrates the initial components for the WeatherNext 2 early access program. It provides essential documentation and a practical Jupyter notebook to enable users to leverage Google DeepMind and Google Research's advanced AI weather forecasting model on Vertex AI, focusing on distributed inference and forecast visualization. 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 introduces a new notebook and documentation for WeatherNext 2. The changes are a good starting point, but I've found several issues that need to be addressed. These include a critical package version typo that will prevent the notebook from running, a broken link for opening in Colab Enterprise, incorrect calculations for GPU requirements that could mislead users, and several smaller issues related to code clarity and document formatting. Please review the detailed comments for suggestions on how to fix these issues.
| "print(\"Installing python packages.\")\n", | ||
| "\n", | ||
| "! pip3 install \\\n", | ||
| " google-cloud-aiplatform==1.129.0 \\\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.
| " </a>\n", | ||
| " </td>\n", | ||
| " <td style=\"text-align: center\">\n", | ||
| " <a href=\"https://console.cloud.google.com/vertex-ai/colab/import/https:%2F%2Fraw.githubusercontent.com%2FGoogleCloudPlatform%2Fvertex-ai-samples%2Fmain%2Fnotebooks%2Fcommunity%weathernext%2Fweathernext_2_early_access_program.ipynb\">\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 "Open in Colab Enterprise" link is broken due to a typo in the URL. A forward slash (/) is missing between community and weathernext in the path, causing the import to fail. The URL needs to be properly encoded.
<a href="https://console.cloud.google.com/vertex-ai/colab/import/https:%2F%2Fraw.githubusercontent.com%2FGoogleCloudPlatform%2Fvertex-ai-samples%2Fmain%2Fnotebooks%2Fcommunity%2Fweathernext%2Fweathernext_2_early_access_program.ipynb">
| "total_gpus_needed = num_samples * (4 if model_seed == \"all\" else 1)\n", | ||
| "\n", | ||
| "# Set Docker URI\n", | ||
| "WEATHERNEXT2_DOCKER_URI = 'us-docker.pkg.dev/vertex-ai-restricted/vertex-vision-model-garden-dockers/weather-next-2-inference.gpu.0-1:latest'\n", | ||
| "\n", | ||
| "print(\"--- Job Configuration Summary ---\")\n", | ||
| "print(f\"Total Samples: {num_samples}\")\n", | ||
| "print(f\"Machine Type: {machine_type}\")\n", | ||
| "print(f\"Accelerator Type: {accelerator_type}\")\n", | ||
| "print(f\"GPUs per Machine: {accelerators_per_machine}\")\n", | ||
| "print(f\"Total number seeds to run: {num_seeds_to_run}\")\n", | ||
| "print(f\"Total number samples per seed: {num_samples_per_seed}\")\n", | ||
| "print(f\"Calculated Machine Replicas Per Seed: {replica_count_per_seed}\")\n", | ||
| "print(f\"Total GPUs per Job: {num_samples}\")\n", | ||
| "print(f\"Total GPUs across all Jobs (ensure sufficient quota): {total_gpus_needed}\")\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 calculation for total_gpus_needed and the values printed in the summary for "Total GPUs per Job" and "Total GPUs across all Jobs" are incorrect.
Total GPUs per Jobshould benum_samples_per_seed.Total GPUs across all Jobsshould benum_samples.
The current logic significantly overestimates the required GPU quota, which could confuse users and lead them to request more resources than necessary.
# Calculate total GPUs needed for all jobs
total_gpus_needed = num_samples
# Set Docker URI
WEATHERNEXT2_DOCKER_URI = 'us-docker.pkg.dev/vertex-ai-restricted/vertex-vision-model-garden-dockers/weather-next-2-inference.gpu.0-1:latest'
print("--- Job Configuration Summary ---")
print(f"Total Samples: {num_samples}")
print(f"Machine Type: {machine_type}")
print(f"Accelerator Type: {accelerator_type}")
print(f"GPUs per Machine: {accelerators_per_machine}")
print(f"Total number seeds to run: {num_seeds_to_run}")
print(f"Total number samples per seed: {num_samples_per_seed}")
print(f"Calculated Machine Replicas Per Seed: {replica_count_per_seed}")
print(f"Total GPUs per Job: {num_samples_per_seed}")
| *Experimental*\ | ||
| This product is subject to the "Pre-GA Offerings Terms" in the General Service Terms section of the [Service Specific Terms](https://cloud.google.com/terms/service-terms#1). Pre-GA products are available "as is" and might have limited support. For more information, see the [launch stage descriptions](https://cloud.google.com/products#product-launch-stages). <!-- disableFinding(LINE_OVER_80) --> | ||
|
|
||
| Access to the forecasting capabilities requires application and approval. Users must be added to an allowlist to generate forecasts using this service. Review pricing details at [Vertex AI Custom Training pricing,](https://cloud.google.com/vertex-ai/pricing?hl=en&e=48754805#custom-trained-models) [Cloud Storage pricing](https://cloud.google.com/storage/pricing), and use the [Pricing Calculator](https://cloud.google.com/products/calculator/) before running. <!-- disableFinding(LINE_OVER_80) --> |
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 a minor typo in the link text for "Vertex AI Custom Training pricing". The comma is included within the link's text, which is unconventional.
| Access to the forecasting capabilities requires application and approval. Users must be added to an allowlist to generate forecasts using this service. Review pricing details at [Vertex AI Custom Training pricing,](https://cloud.google.com/vertex-ai/pricing?hl=en&e=48754805#custom-trained-models) [Cloud Storage pricing](https://cloud.google.com/storage/pricing), and use the [Pricing Calculator](https://cloud.google.com/products/calculator/) before running. <!-- disableFinding(LINE_OVER_80) --> | |
| Access to the forecasting capabilities requires application and approval. Users must be added to an allowlist to generate forecasts using this service. Review pricing details at [Vertex AI Custom Training pricing](https://cloud.google.com/vertex-ai/pricing?hl=en&e=48754805#custom-trained-models), [Cloud Storage pricing](https://cloud.google.com/storage/pricing), and use the [Pricing Calculator](https://cloud.google.com/products/calculator/) before running. <!-- disableFinding(LINE_OVER_80) --> |
|
|
||
| Access to the forecasting capabilities requires application and approval. Users must be added to an allowlist to generate forecasts using this service. Review pricing details at [Vertex AI Custom Training pricing,](https://cloud.google.com/vertex-ai/pricing?hl=en&e=48754805#custom-trained-models) [Cloud Storage pricing](https://cloud.google.com/storage/pricing), and use the [Pricing Calculator](https://cloud.google.com/products/calculator/) before running. <!-- disableFinding(LINE_OVER_80) --> | ||
|
|
||
| **Overview** |
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 document contains several redundant subheadings. For example, under the ## Overview heading on line 4, there is another bolded **Overview** line here. This pattern repeats for "Use Cases" (line 20), "Training data" (line 31), and others. These are unnecessary and can be removed to improve readability.
| Running these models *will incur costs* for the GPUs and other Google Cloud resources used. Learn about [Vertex AI Custom Training pricing](https://cloud.google.com/vertex-ai/pricing?hl=en&e=48754805#custom-trained-models), [Cloud Storage pricing](https://cloud.google.com/storage/pricing), and use the [Pricing Calculator](https://cloud.google.com/products/calculator/) to generate an estimate. | ||
|
|
||
| ## Quick start | ||
| The quickest way to get started with the WeatherNext in Google Cloud Platform is to run [our example notebook](weathernext_2_early_access_program.ipynb) in [Google Colab](https://colab.research.google.com/). |
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 link to "Google Colab" is generic. It would be more user-friendly to provide a direct link that opens this specific notebook in Colab, similar to the "Open in Colab" button at the top of the notebook itself.
| The quickest way to get started with the WeatherNext in Google Cloud Platform is to run [our example notebook](weathernext_2_early_access_program.ipynb) in [Google Colab](https://colab.research.google.com/). | |
| The quickest way to get started with the WeatherNext in Google Cloud Platform is to run [our example notebook](weathernext_2_early_access_program.ipynb) in [Google Colab](https://colab.research.google.com/github/GoogleCloudPlatform/vertex-ai-samples/blob/main/notebooks/community/weathernext/weathernext_2_early_access_program.ipynb). |
| "! gcloud services enable aiplatform.googleapis.com compute.googleapis.com\n", | ||
| "\n", | ||
| "# Cloud Storage bucket for storing the experiment artifacts.\n", | ||
| "now = datetime.datetime.now().strftime(\"%Y%m%d%H%M%S\")\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.
| "if num_samples_per_seed % accelerators_per_machine != 0:\n", | ||
| " raise ValueError(f\"`num_samples_per_seed` ({num_samples_per_seed}) must be a multiple of the GPUs per machine ({accelerators_per_machine} for {machine_type}).\")\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.
| "launched_jobs = []\n", | ||
| "output_dirs = {}\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 variables launched_jobs and output_dirs are initialized here but their values are never used. This is dead code and can be removed.
You should also remove the lines where they are assigned values inside the loop (output_dirs[seed] = output_dir on line 295 and launched_jobs.append(job) on line 325) to clean up the code.
REQUIRED: Add a summary of your PR here, typically including why the change is needed and what was changed. Include any design alternatives for discussion purposes.
--- YOUR PR SUMMARY GOES HERE ---
REQUIRED: Fill out the below checklists or remove if irrelevant
2. If you are opening a PR for
Community Notebooksunder the notebooks/community folder:Community Notebookssection, pointing to the author or the author's team.