examples: First complete draft of dispersion notebook

[JDBetteridge]

[review-notebook-app[bot]]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[codecov[bot]]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 92.14%. Comparing base (5111506) to head (4809a0e). :warning: Report is 9 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2595   +/-   ##
=======================================
  Coverage   92.14%   92.14%           
=======================================
  Files         248      248           
  Lines       50022    50022           
  Branches     4402     4402           
=======================================
  Hits        46091    46091           
  Misses       3222     3222           
  Partials      709      709           

Flag	Coverage Δ
pytest-gpu-aomp-amdgpuX	70.90% <ø> (+0.01%)	:arrow_up:
pytest-gpu-nvc-nvidiaX	71.87% <ø> (+0.01%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

georgebisbas commented on 2025-05-02T06:30:10Z ----------------------------------------------------------------

I think we usually have references at the last cell of a notebook. Not a strict rule though.

JDBetteridge commented on 2025-05-02T12:36:06Z ----------------------------------------------------------------

Fixed

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

georgebisbas commented on 2025-05-02T06:30:11Z ----------------------------------------------------------------

Line #1.    # We import everything we need for the noterbook at the start

notebook typo

JDBetteridge commented on 2025-05-02T12:36:08Z ----------------------------------------------------------------

Comment was superfluous anyway

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

georgebisbas commented on 2025-05-02T06:30:12Z ----------------------------------------------------------------

Should you add a comment on how it is (if it is) different from Deviot's builtin Ricker?

JDBetteridge commented on 2025-05-02T12:36:09Z ----------------------------------------------------------------

It isn't, but I really don't think it's necessary to invoke the wrath of examples.seismic.source.py that whole file could be 5 lines IMHO. I will change it if I'm instructed to, but this is much clearer

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

georgebisbas commented on 2025-05-02T06:30:12Z ----------------------------------------------------------------

Citation can be a link I guess

JDBetteridge commented on 2025-05-02T12:36:14Z ----------------------------------------------------------------

Fixed

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

georgebisbas commented on 2025-05-02T06:30:13Z ----------------------------------------------------------------

receiver*

JDBetteridge commented on 2025-05-02T12:36:17Z ----------------------------------------------------------------

Yes!

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

georgebisbas commented on 2025-05-02T06:30:14Z ----------------------------------------------------------------

Courant*

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

georgebisbas commented on 2025-05-02T06:30:15Z ----------------------------------------------------------------

Line #2.    # space as per the work of Tam and Webb, Caunt

Citations can be lifted to previous cell, and add link

JDBetteridge commented on 2025-05-02T12:36:26Z ----------------------------------------------------------------

Superfluous comment anyway

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

georgebisbas commented on 2025-05-02T06:30:16Z ----------------------------------------------------------------

velocity*

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

georgebisbas commented on 2025-05-02T06:30:16Z ----------------------------------------------------------------

Indistinguishable?*

JDBetteridge commented on 2025-05-02T12:36:41Z ----------------------------------------------------------------

Also see* :joy:

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

mloubout commented on 2025-05-02T12:33:42Z ----------------------------------------------------------------

Nitpicking: We usually group the imports as

lib

external

devito

so here would be

from functools import partial

import numpy as np
import scipy as sp
import sympy as sym
from matplotlib import patheffects
import matplotlib.colors as colors
import matplotlib.pyplot as plt

from devito import Grid, Function, TimeFunction, SparseTimeFunction, Eq, Operator, solve

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

mloubout commented on 2025-05-02T12:33:43Z ----------------------------------------------------------------

THis should be available in examples/seismic/sources.py , maybe make it an importable function

JDBetteridge commented on 2025-09-10T00:14:34Z ----------------------------------------------------------------

This is going to be addressed in a refactor in a separate PR

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

mloubout commented on 2025-05-02T12:33:44Z ----------------------------------------------------------------

Spectrum (Fourier transform) is usualy plotted as continuous in geophys, so just plot not bar

EdCaunt commented on 2025-06-02T11:01:47Z ----------------------------------------------------------------

I quite like it as bars. It more clearly identifies the harmonics. I think conflating spectrum and spectral envelope is quite confusing really and probably bad practice, even if it is commonplace

mloubout commented on 2025-08-05T12:43:38Z ----------------------------------------------------------------

It's not clearer, people, especially the target audience, do not look at the Fourier spectrum like that, but as line plots. This makes it confusing. It's commonplace for a reason.

JDBetteridge commented on 2025-09-10T00:14:55Z ----------------------------------------------------------------

Fixed

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

mloubout commented on 2025-05-02T12:33:45Z ----------------------------------------------------------------

Maybe point to where it's computed in examples/seismic/model.py so user/reader see it's implemented in the examples.

Also maybe h_min in case someone use different spacing in different dimensions

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

mloubout commented on 2025-05-02T12:33:45Z ----------------------------------------------------------------

Never seen that left one, that's interesting.

EdCaunt commented on 2025-05-02T14:10:45Z ----------------------------------------------------------------

It corresponds to the "squaring" effect of numerical dispersion on an expanding wavefront (effective velocity is greater off-axis and less on-axis).

[JDBetteridge]

Fixed

---

View entire conversation on ReviewNB

[JDBetteridge]

Comment was superfluous anyway

---

View entire conversation on ReviewNB

[JDBetteridge]

It isn't, but I really don't think it's necessary to invoke the wrath of examples.seismic.source.py that whole file could be 5 lines IMHO. I will change it if I'm instructed to, but this is much clearer

---

View entire conversation on ReviewNB

[JDBetteridge]

Fixed

---

View entire conversation on ReviewNB

[JDBetteridge]

Yes!

---

View entire conversation on ReviewNB

[JDBetteridge]

Superfluous comment anyway

---

View entire conversation on ReviewNB

[JDBetteridge]

Also see* :joy:

---

View entire conversation on ReviewNB

[EdCaunt]

It corresponds to the "squaring" effect of numerical dispersion on an expanding wavefront (effective velocity is greater off-axis and less on-axis).

---

View entire conversation on ReviewNB

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

EdCaunt commented on 2025-05-02T14:20:18Z ----------------------------------------------------------------

Typo: "peeking" (to surreptitiously look at something), not "peaking" (to reach a highest point)

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

EdCaunt commented on 2025-05-02T14:20:19Z ----------------------------------------------------------------

Line #12.            print(f'Optimal weights for {ii}{th} derivative on the {jj + ii + 1} point(s) x = {x[:jj + ii + 1]}:\n\t {w[:jj + ii + 1]}')

Nitpick: Split this print over two lines.

Have you flake8 linted the notebook with nbqa?

JDBetteridge commented on 2025-07-31T11:29:09Z ----------------------------------------------------------------

I assume you mean 3 lines, it's already split over 2

No, none of the other notebooks are linted

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

EdCaunt commented on 2025-05-02T14:20:20Z ----------------------------------------------------------------

Nitpick: "Investigation of dispersion properties" would read more naturally

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

EdCaunt commented on 2025-05-02T14:20:21Z ----------------------------------------------------------------

I presume it assumes an antisymmetric stencil for first derivatives, or is this analysis not applicable?

JDBetteridge commented on 2025-07-31T11:32:14Z ----------------------------------------------------------------

This is only looking at second derivatives (ie: only this problem), covering the theory for first derivatives would probably be too much, this notebook is already massive

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

EdCaunt commented on 2025-05-02T14:20:21Z ----------------------------------------------------------------

Typo: "missfit" -> "misfit"

Also this paragraph is pretty garbled and needs a rewrite

JDBetteridge commented on 2025-07-31T11:35:52Z ----------------------------------------------------------------

I think this was a pasting error

[review-notebook-app[bot]]

View / edit / reply to this conversation on ReviewNB

EdCaunt commented on 2025-05-02T14:20:22Z ----------------------------------------------------------------

This will need an #NBVAL_IGNORE_OUTPUT

JDBetteridge commented on 2025-07-31T11:40:07Z ----------------------------------------------------------------

nbval-ignore-output is in the metadata