Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jWave vs kWave unit tests #33

Merged
merged 30 commits into from
May 14, 2022
Merged

jWave vs kWave unit tests #33

merged 30 commits into from
May 14, 2022

Conversation

btreeby
Copy link
Member

@btreeby btreeby commented May 11, 2022

This pull request implements a systematic test of time_varying.py, in particular the simulate_wave_propagation solver. A variety of simulations are setup:

  • with / without PML
  • p0 / time varying sources
  • homogeneous / heterogeneous c0 and rho0
  • even and odd grid sizes

The tests are implemented in two files:

  • test_kwave_ivp for initial value problems (p0 source)
  • test_kwave_tvsp for time varying mass sources (p source)

On a Linux or Mac system with MATLAB installed and accessible via the matlab command, and k-Wave on the default path, the test scripts will also run the examples in k-Wave to generate the test data.

As part of the development of the tests, and number of bug fixes where implemented (see also #36).

  • Correctly set u0 to give u0(t=0) = 0 for initial value problems (was set to zero)
  • Correctly calculate rho0 on the staggered grid (interpolation was in wrong direction)
  • Correctly calculate source scaling for time varying mass sources (missing factors)
  • Correctly calculate PML on the staggered grid (ignored grid staggered)

Closes #31
Closes #34

@btreeby btreeby self-assigned this May 11, 2022
@codecov
Copy link

codecov bot commented May 11, 2022

Codecov Report

Merging #33 (e44d179) into main (b293d95) will increase coverage by 3.99%.
The diff coverage is 77.22%.

@@            Coverage Diff             @@
##             main      #33      +/-   ##
==========================================
+ Coverage   43.76%   47.75%   +3.99%     
==========================================
  Files          15       15              
  Lines         866      936      +70     
==========================================
+ Hits          379      447      +68     
- Misses        487      489       +2     
Impacted Files Coverage Δ
jwave/utils.py 20.89% <25.00%> (-20.14%) ⬇️
jwave/geometry.py 61.84% <95.65%> (+10.66%) ⬆️
jwave/acoustics/pml.py 73.13% <96.15%> (+12.66%) ⬆️
jwave/acoustics/time_varying.py 87.83% <100.00%> (+5.07%) ⬆️
jwave/signal_processing.py 29.16% <0.00%> (+5.20%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b293d95...e44d179. Read the comment docs.

@btreeby btreeby changed the title Draft: kWave Comparisons jWave vs kWave unit tests May 13, 2022
@btreeby btreeby requested a review from astanziola May 13, 2022 11:45
@astanziola
Copy link
Member

Closes #28

@@ -215,53 +216,69 @@ def simulate_wave_propagation(
checkpoint: bool = False,
params = None,
smooth_initial = True,
return_params = False,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm removing this input, as the optional output of the parameters is already handled by jaxdf within an opeator decorator.

@astanziola astanziola merged commit 7fff3b7 into main May 14, 2022
@astanziola astanziola deleted the optm_kwave_comparison branch June 6, 2022 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The time_varying field output is two steps ahead of k-wave Set symmetric velocity conditions for u0
2 participants