ucl-bug/jwave

Implementing off-grid sensors

Closed this issue · 4 comments

tomelse commented

Hi! I thought it would be really nice to introduce off-grid sensors into j-Wave, as they have done in k-Wave (http://www.k-wave.org/forum/topic/alpha-version-of-kwavearray-off-grid-sources).

I've implemented a basic version of this myself using the band-limited interpolant, which you can see in this gist: https://gist.github.com/tomelse/f9ba7508b75f44f34ebdbf25d5f5b0a3.

I'm happy to put a pull request in myself, I just wanted to get some feedback on how best to integrate it into the j-Wave code.

bli_comparison

The results look really nice, removing all the staircasing that you can see in the on-grid version, which you can see in an example simulation above (x axis = time, y axis= detector). It's a little bit slower than the original j-Wave implementation but not prohibitively so (maybe that's to be expected?).

astanziola commented

Hello Thomas. That looks great, thanks a lot! Please do feel absolutely free to open a PR, it would be amazing to have off-grid sensors.

I think this can easily go into the geometry.py file almost as is, although there may be a good argument to start having two separate sources and sensors modules.

I was looking at the comment given by @jgroehl below your gist and I think I agree with him. I can imagine a situation when someone want so do something like

def my_loss_func(sensors_positions: Tuple[np.typing.ArrayLike]:
    sensors = BLISensors(sensors_positions)
    ...
    
sensor_positions_grad = jax.grad(my_loss_func)

In that way the position of the sensors can also be tuned by gradient descent! In that case, the flattening and unflattening methods also need to be changed in the pytree definition.

It would also be good to add some testing for this, for example by reproducing the followings:

But the way the testing is handled in this repo currently is a bit clunky (any suggestion is more than welcome!), so I'm happy to add them afterwards

Thank you for taking time to do this and share it!h

astanziola commented

Hello @tomelse ! Just gauging if you think you would like to make a PR on this? Otherwise I'm happy to take this over (of course acknowledging your contribution!)

tomelse commented

I'll have a look next week, had such a hectic week!

tomelse commented

@astanziola I've implemented this in a new pull request (#192). There's example code, test code and an updated implementation.

  • ❤️1