Issue 0127 moment source#141
Conversation
…elastic wave to allow comparisons with devito/branch A
…m camembert elastic example
|
What still needs to be done to merge this? |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #141 +/- ##
==========================================
- Coverage 84.63% 82.50% -2.13%
==========================================
Files 70 72 +2
Lines 5687 5848 +161
==========================================
+ Hits 4813 4825 +12
- Misses 874 1023 +149 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This pull request implements support for moment source functionality (closes #127) and includes significant updates to source handling, stability checking, and 3D meshing capabilities.
- Adds integral mode for Ricker wavelet to support moment tensor sources
- Refactors stability checking into solver-specific implementations
- Implements 3D mesh generation using SeismicMesh
- Adds comprehensive analytical solution example for elastic wave validation
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/on_one_core/test_sources.py | Updated test calls to use keyword argument for amplitude parameter |
| test_3d/test_elastic_isotropic.py | Added parametrized tests for force source validation against analytical solutions |
| spyro/utils/file_utils.py | Added utility function for creating timestamped directories |
| spyro/sources/Sources.py | Added integral mode to Ricker wavelet, refactored wavelet initialization order, and changed parameter signatures |
| spyro/solvers/wave.py | Added abstract method for stability checking |
| spyro/solvers/time_integration_central_difference.py | Refactored stability check to use solver-specific implementation |
| spyro/solvers/elastic_wave/isotropic_wave.py | Implemented stability check using finite norm check for elastic solver |
| spyro/solvers/acoustic_wave.py | Implemented stability check using norm threshold for acoustic solver |
| spyro/receivers/dirac_delta_projector.py | Removed debug print statements |
| spyro/meshing/meshing_functions.py | Implemented 3D homogeneous mesh generation and removed commented code |
| spyro/examples/elastic_analytical.py | Added comprehensive example with analytical solution validation for elastic waves |
| spyro/examples/camembert_elastic.py | Updated timing parameters for more efficient output |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def create_seismicmesh_3d_mesh(self): | ||
| """ | ||
| Creates a 3D mesh based on SeismicMesh meshing utilities. | ||
| """ | ||
| if self.velocity_model is None: | ||
| return self.create_seismicmesh_3D_mesh_homogeneous() |
There was a problem hiding this comment.
Inconsistent naming convention: the function create_seismicmesh_3d_mesh uses lowercase 3d while it calls create_seismicmesh_3D_mesh_homogeneous which uses uppercase 3D. The naming should be consistent - either both should use lowercase 3d or both should use uppercase 3D.
| tt = (math.pi * freq * t) ** 2 | ||
| return amp * (1.0 - (2.0) * tt) * math.exp((-1.0) * tt) | ||
| if integral: | ||
| return t*math.exp((-1.0) * tt) * amplitude |
There was a problem hiding this comment.
Missing space around * operator. According to PEP 8, binary operators should be surrounded by a single space on both sides. The expression should be t * math.exp((-1.0) * tt) * amplitude.
| return t*math.exp((-1.0) * tt) * amplitude | |
| return t * math.exp((-1.0) * tt) * amplitude |
| def ricker_wavelet( | ||
| t, freq, amp=1.0, delay=1.5, delay_type="multiples_of_minimun" | ||
| t, freq, delay=1.5, delay_type="multiples_of_minimun", | ||
| integral=False, amplitude=1.0, |
There was a problem hiding this comment.
Missing documentation for new parameters integral and amplitude. The docstring should include descriptions for these parameters that were added to the function signature.
|
|
||
| def ricker_wavelet( | ||
| t, freq, amp=1.0, delay=1.5, delay_type="multiples_of_minimun" | ||
| t, freq, delay=1.5, delay_type="multiples_of_minimun", |
There was a problem hiding this comment.
Typo in condition check: "minimun" should be "minimum". This spelling error appears in the string comparison.
| cutoff=None, | ||
| delay=1.5, | ||
| delay_type="multiples_of_minimun", | ||
| integral=False |
There was a problem hiding this comment.
Missing documentation for new parameter integral. The docstring should include a description for this parameter that was added to the function signature.
| } | ||
|
|
||
| wave = spyro.IsotropicWave(d) | ||
| wave.set_mesh(mesh_parameters={'edge_length': h}) |
There was a problem hiding this comment.
Keyword argument 'mesh_parameters' is not a supported parameter name of method Wave.set_mesh.
| wave.set_mesh(mesh_parameters={'edge_length': h}) | |
| wave.set_mesh(edge_length=h) |
| @@ -0,0 +1,39 @@ | |||
| import math | |||
| import numpy as np | |||
There was a problem hiding this comment.
Import of 'np' is not used.
| import numpy as np |
|
Remove the section that keeps creating Vs |
Closes #127