Duplicate the last layer of the HF 1D VM to stop crashes with subduction runs#84
Duplicate the last layer of the HF 1D VM to stop crashes with subduction runs#84lispandfound merged 5 commits intopegasusfrom
Conversation
…es with subduction runs
Summary of ChangesHello @lispandfound, 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 addresses a critical crash encountered in subduction simulations when the High-Frequency 1D Velocity Model's raycasting mechanism attempts to access beyond its defined layers. The resolution involves a straightforward modification to the default parameter YAML files, specifically by duplicating the last velocity layer. This ensures that the model has an extended, identical layer to prevent index out-of-bounds errors without altering the core Fortran implementation, thereby enhancing the robustness of the simulation workflow. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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.
Code Review
This pull request addresses a crash in subduction runs by duplicating the last layer of the HF 1D velocity model in several default parameter files. This is a pragmatic workaround for an issue in the underlying Fortran code. My review focuses on improving the maintainability of this change. I've suggested adding comments to the YAML files to explain why the layer is duplicated, which will prevent future confusion or accidental removal of this necessary workaround. The changes are otherwise correct and consistent across the relevant configuration files.
There was a problem hiding this comment.
Pull request overview
This PR fixes crashes in HF raycasting during subduction runs by duplicating the last layer of the 1D velocity model. The issue occurs when HF raycasting travels through the last layer, and the simplest fix is to duplicate this layer rather than modifying the HF Fortran code.
Key changes:
- Duplicated the last layer (thickness: 999.999, Vp: 8.10, Vs: 4.60, rho: 3.33, Qp: 394.80, Qs: 197.40) in the
hf_velocity_model_1dsection across multiple version configurations - Added YAML document separator (
---) to three version files for consistency
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| workflow/default_parameters/v24_2_2_4/defaults.yaml | Duplicates the last HF velocity model layer and adds YAML document separator |
| workflow/default_parameters/v24_2_2_2/defaults.yaml | Duplicates the last HF velocity model layer and adds YAML document separator |
| workflow/default_parameters/v24_2_2_1/defaults.yaml | Duplicates the last HF velocity model layer and adds YAML document separator |
| workflow/default_parameters/develop/defaults.yaml | Duplicates the last HF velocity model layer (no YAML separator added) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
sungeunbae
left a comment
There was a problem hiding this comment.
Looks like the best solution for now.
Pretty simple, noticed by @claudio525 first in his subduction runs. If the HF raycasting requires travels through the last layer then the code will crash. The simplest fix that doesn't require changing the HF fortran code is just duplicating the last layer. I believe I saw this when porting from the old workflow and assumed it was a typo... It doesn't have any impact on simulations that do not propagate to the last layer (i.e. most of them)