Add trimming module to source modelling#52
Conversation
There was a problem hiding this comment.
Summary of Changes
Hello @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 introduces a new module dedicated to refining earthquake source models by trimming slip arrays. The functionality aims to improve the physical consistency and accuracy of models, particularly those based on inversion, by removing low-slip regions from oversized fault dimensions. This enhancement helps align model outputs with established geophysical relationships.
Highlights
- New Trimming Module: Introduces a new
trimmodule withinsource_modellingto handle slip array trimming, essential for refining earthquake source models. - Implemented Trimming Methods: Adds two distinct slip trimming methodologies: the Sommerville method and the Thingbaijam method, both designed to generate boolean masks for slip arrays based on scientific criteria.
- Utility Functions: Includes utility functions such as
autocorrelation_dimensionandtrim_array_to_target_lengthto support the main trimming algorithms by calculating characteristic lengths and spatial boundaries. - Comprehensive Testing: Adds a new test file
tests/test_trim.pywith comprehensive unit and property-based tests to ensure the correctness and robustness of all new trimming functionalities.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request adds a new trim module to source_modelling for trimming slip arrays, which is a valuable addition. The code includes two trimming methods, sommerville and thingbaijam, along with unit tests. My review focuses on improving correctness and maintainability. I've identified a few issues, including a potential division-by-zero error, a logic bug in one of the trimming helper functions, and several opportunities to improve code clarity and adhere to Python conventions. The provided tests are a good start, but they don't catch the logic bug I've pointed out. Addressing these points will make the new module more robust and easier to maintain.
|
Had to appease the type checker with some minor tweaks to return types. |
There was a problem hiding this comment.
Pull Request Overview
This pull request adds a new trimming module to source modelling for analyzing and trimming slip arrays on fault models. The module provides two trimming methods (Somerville and Thingbaijam) to improve performance of source models against common seismological relationships.
- Implements Somerville and Thingbaijam slip trimming methods for removing low-slip regions
- Adds autocorrelation dimension calculations for characteristic length scale determination
- Includes comprehensive test coverage with property-based testing using Hypothesis
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| source_modelling/trim.py | New module implementing slip array trimming algorithms and autocorrelation calculations |
| tests/test_trim.py | Comprehensive test suite for the trimming module with unit and property-based tests |
| tests/test_sources.py | Minor update to use shapely.equals instead of geometry.equals method |
| tests/test_gsf.py | Minor updates to use shapely.contains instead of geometry.contains method |
| source_modelling/sources.py | Code formatting improvements and type annotation update for geometry property |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
AndrewRidden-Harper
left a comment
There was a problem hiding this comment.
looks good. I made a couple of suggestions for comments
| ): | ||
| # Here at least one of slip_function[left] < keep_threshold or | ||
| # slip_function[right - 1] < keep_threshold is true. Hence, we | ||
| # just need to check which smallest, as that one *definitely* |
There was a problem hiding this comment.
| # just need to check which smallest, as that one *definitely* | |
| # just need to check which is smallest, as that one *definitely* |
| mask = trim.trim_mask_thingbaijam(slip, dx, dz, keep_top=True) | ||
|
|
||
| # With keep_top=True, the top edge of the region is kept | ||
| # So the mask will include all rows from top of region down |
There was a problem hiding this comment.
| # So the mask will include all rows from top of region down | |
| # So the mask will include all rows from the top of the region down |
Adds a module for trimming slip arrays to source modelling.
Slip trimming is needed for SRCMOD models (and other inversion based models) because researchers will often impose a much larger dimension on the fault than is physical. Trimming improves the performance of the models against common relationships (Mw-Area, Avg. slip, geometry based methods like RRup-IM, RJB-IM, etc).
Both the implemented methods (the basic Sommerville method, and the Thingbaijam method) take slip arrays in the shape (nstk, ndip) and produce boolean masks over the arrays. So
slip[mask]will return a new array containing only the masked slip.