Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request enhances the model smoothing functionality and adds utility functions for distance-angle conversions. The changes include new km/degree conversion helpers and a significantly refactored smooth() method that provides better control over smoothing parameters with support for different units and anisotropic parameter smoothing.
- Added
km2deg()anddeg2km()utility functions for distance-angle conversions - Enhanced the
smooth()method with latitude-aware longitude corrections, unit flexibility (degrees/km), and anisotropic parameter smoothing support - Cleaned up commented-out code in
to_ani()method - Version bump to 0.2.11 and codecov badge URL update
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pytomoatt/utils/common.py | Adds km2deg and deg2km conversion functions using spherical Earth approximation |
| pytomoatt/model.py | Refactors smooth() method with new parameters (unit_deg, smooth_ani), adds km2deg import, removes commented code |
| pytomoatt/_version.py | Version bump from 0.2.9 to 0.2.11 |
| README.md | Updates codecov badge URL to new repository organization |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
Comment on lines
+136
to
191
| def smooth(self, sigma=5.0, unit_deg=False, smooth_ani=False, **kwargs): | ||
| """Gaussian smooth the 3D velocity model | ||
|
|
||
| :param sigma: Standard division of gaussian kernel in km, defaults to 10 | ||
| :type sigma: scalar or sequence of scalars , optional | ||
| :param sigma: Standard deviation for Gaussian kernel. | ||
| If scalar, apply to all dimensions. | ||
| If sequence of 3, apply to [depth, lat, lon]. | ||
| Depth is always in km. | ||
| Horizontal dimensions depend on unit_deg. | ||
| :type sigma: scalar or sequence of scalars | ||
| :param unit_deg: If True, horizontal sigma is in degrees. | ||
| If False, horizontal sigma is in km. | ||
| Defaults to False. | ||
| :type unit_deg: bool | ||
| :param smooth_ani: If True, also smooth anisotropic parameters (xi, eta, zeta). | ||
| Defaults to False. | ||
| :type smooth_ani: bool | ||
| :param kwargs: Additional arguments passed to scipy.ndimage.gaussian_filter | ||
|
|
||
| Example | ||
| ------------------- | ||
| To smooth with 5 km in depth and 0.2 degrees in horizontal directions: | ||
| >>> model.smooth(sigma=[5.0, 0.2, 0.2], unit_deg=True) | ||
|
|
||
| To smooth with 5 km in depth and 20 km in horizontal directions: | ||
| >>> model.smooth(sigma=[5.0, 20.0, 20.0], unit_deg=False) | ||
| """ | ||
| if isinstance(sigma, (int, float)): | ||
| sigma_all = np.ones(3)*sigma/self.d_rtp/2/np.pi | ||
| elif len(sigma) == 3: | ||
| sigma_all = np.array(sigma)/self.d_rtp/2/np.pi | ||
| sigma_all[0:2] /= 111.19 | ||
| self.vel = gaussian_filter(self.vel, sigma) | ||
| if np.isscalar(sigma): | ||
| sigma = [sigma, sigma, sigma] | ||
| elif len(sigma) != 3: | ||
| raise ValueError('sigma should be a scalar or a sequence of three scalars') | ||
|
|
||
| sigma = np.array(sigma, dtype=float) | ||
| sigma_pixel = np.zeros(3) | ||
|
|
||
| # Depth direction (always km) | ||
| sigma_pixel[0] = sigma[0] / self.d_rtp[0] | ||
|
|
||
| if unit_deg: | ||
| # Horizontal sigma is in degrees | ||
| sigma_pixel[1] = sigma[1] / self.d_rtp[1] | ||
| sigma_pixel[2] = sigma[2] / self.d_rtp[2] | ||
| else: | ||
| # Horizontal sigma is in km | ||
| # Latitude | ||
| sigma_pixel[1] = km2deg(sigma[1]) / self.d_rtp[1] | ||
| # Longitude | ||
| mean_lat = np.mean(self.latitudes) | ||
| # 1 deg lon = cos(lat) * 1 deg lat | ||
| # so X km = km2deg(X) deg lat = km2deg(X) / cos(lat) deg lon | ||
| sigma_pixel[2] = km2deg(sigma[2]) / np.cos(np.deg2rad(mean_lat)) / self.d_rtp[2] | ||
| self.vel = gaussian_filter(self.vel, sigma_pixel, **kwargs) | ||
| if smooth_ani: | ||
| self.xi = gaussian_filter(self.xi, sigma_pixel, **kwargs) | ||
| self.eta = gaussian_filter(self.eta, sigma_pixel, **kwargs) | ||
| self.zeta = gaussian_filter(self.zeta, sigma_pixel, **kwargs) | ||
|
|
There was a problem hiding this comment.
The enhanced smooth() method with new parameters (unit_deg, smooth_ani) and significantly refactored logic lacks test coverage. Given that the repository has comprehensive tests for model operations (test/test_create_model.py), tests should be added for:
- Scalar vs. sequence sigma handling
unit_deg=Truevsunit_deg=Falsebehaviorsmooth_ani=Trueto verify xi, eta, zeta are smoothed- Longitude correction with latitude dependency
- ValueError for invalid sigma length
… km2deg/deg2km conversions
… for utility functions
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.