Fix plot_perturbation to ensure nbl is not plotted #2797
Conversation
Fix plotting of nbl
EdCaunt
left a comment
EdCaunt
left a comment
There was a problem hiding this comment.
Minor comment, but generally a useful fix. Thanks!
examples/seismic/plotting.py
Outdated
| dv = np.transpose(model.vp.data) - np.transpose(model1.vp.data) | ||
| slices = tuple(slice(model.nbl, -model.nbl) for _ in range(2)) | ||
| slices1 = tuple(slice(model1.nbl, -model1.nbl) for _ in range(2)) | ||
| dv = np.transpose(model.vp.data[slices]) - np.transpose(model1.vp.data[slices1]) |
There was a problem hiding this comment.
I would personally check model.nbl == model1.nbl and raise a suitable ValueError if not (probably also check the size of both models match whilst you're at it).
That way you only need a single slices which would be tidier and easier to read in my opinion.
Minor note, you can also do slices = (slice(model.nbl, -model.nbl),) * 2 for concision, although this is arguably less explicit. A matter of personal preference I would say.
examples/seismic/plotting.py
Outdated
| domain_size = 1.e-3 * np.array(model.domain_size) | ||
| extent = [model.origin[0], model.origin[0] + domain_size[0], | ||
| model.origin[1] + domain_size[1], model.origin[1]] | ||
| assert model.nbl == model1.nbl, ValueError("model and model1 have different values for nbl") |
There was a problem hiding this comment.
Use an fstring here so you get the actual names of the model objects
examples/seismic/plotting.py
Outdated
| extent = [model.origin[0], model.origin[0] + domain_size[0], | ||
| model.origin[1] + domain_size[1], model.origin[1]] | ||
| dv = np.transpose(model.vp.data) - np.transpose(model1.vp.data) | ||
| assert model.nbl == model1.nbl, ValueError("model and model1 have different values for nbl") |
There was a problem hiding this comment.
the correct synthax is
if model.nbl != model1.nbl:
raise ValueError("model and model1 have different values for nbl")
the second arg of assert a, b is a plain string passed to AssertionError
EdCaunt
left a comment
EdCaunt
left a comment
There was a problem hiding this comment.
I'm happy with this once CI is green
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2797 +/- ##
==========================================
- Coverage 82.98% 78.97% -4.01%
==========================================
Files 248 248
Lines 51012 50823 -189
Branches 4491 4391 -100
==========================================
- Hits 42332 40139 -2193
- Misses 7909 9881 +1972
- Partials 771 803 +32
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3 participants
Description
plot_perturbationhas a small bug where the nbl is not properly addressed when plotting. This PR addressed that by applying the same slicing asplot_velocity.