odlgroup / odl

Operator Discretization Library https://odlgroup.github.io/odl/
Mozilla Public License 2.0
368 stars 105 forks source link

ASTRA FBP #185

Closed adler-j closed 8 years ago

adler-j commented 8 years ago

As mentioned by @sseebbbbee in issue #172, access to astras FBP is needed. To do this, we need to create a new class (probably very similar to the back-projection class) that allows this.

FBP as ForwardProjector.inverse

We also had a discussion on whether or not to add this as a inverse property to the forward projection. A short pro-con:

Pros
kohr-h commented 8 years ago

Often considered the inverse in the litterature e.g. wikipedia

You're mixing up things here. The inversion formula is of FBP type, of course, but that's true for many related invertible transforms. When people talk of filtered backprojection, however, they usually don't mean the unregularized version of it. If we choose to implement the exact inversion formula as an FBP method with the ramp filter, fine. But if we choose a regularizing filter, we're not inverting.

While we have made a couple of compromises where we chose practicability in favor of mathematical rigor, we never did anything plain wrong. This would be the first time, and I think we shouldn't go down that route.

I suggest that we either make an approx_inverse method which allows to set some parameters (e.g. filter function, filter width, ...) or add FBP as a solver. In the first case, we could then (compromise!) use some filter with extremely small width as inverse.

adler-j commented 8 years ago

You're mixing up things here. The inversion formula is of FBP type, of course, but that's true for many related invertible transforms. When people talk of filtered backprojection, however, they usually don't mean the unregularized version of it. If we choose to implement the exact inversion formula as an FBP method with the ramp filter, fine. But if we choose a regularizing filter, we're not inverting.

Agreed

While we have made a couple of compromises where we chose practicability in favor of mathematical rigor, we never did anything plain wrong. This would be the first time, and I think we shouldn't go down that route.

Valid point

I suggest that we either make an approx_inverse method which allows to set some parameters (e.g. filter function, filter width, ...) or add FBP as a solver. In the first case, we could then (compromise!) use some filter with extremely small width as inverse.

Agreed on both points I guess. Simply setting the underlying filter to be a ramp filter (i guess how to do this depends on the underlying library) and calling it inverse should be fine to me.

adler-j commented 8 years ago

On reading about the astra FBP it seems to be too narrow for our use. Propose close this and implement issue #228 and use RTK for this instead.

kohr-h commented 8 years ago

Hm, dunno. That's another dependency, and it would be somewhat sad if we offered an approx_inverse method which only works if you additionally installed some other package. Surely it's not highest prio, but if we settle for this way I think we should offer this option.

How limiting is it exactly? Are there many setups we cannot run at all or can we just close our eyes and have astra hammer on the data? With a big disclaimer, that should be fine (we could even weaken the approx part to coarse, dirty, quick or similar).

adler-j commented 8 years ago

Hm, dunno. That's another dependency, and it would be somewhat sad if we offered an approx_inverse method which only works if you additionally installed some other package. Surely it's not highest prio, but if we settle for this way I think we should offer this option.

Sure the best would be if Astra extended their functionality.

How limiting is it exactly? Are there many setups we cannot run at all or can we just close our eyes and have astra hammer on the data? With a big disclaimer, that should be fine (we could even weaken the approx part to coarse, dirty, quick or similar).

Astra requires a centered detector for its FBP implementation, this is not even remotely true for any of the geometries I know.

The only way I can see to solve this would be to resample the data and add zeros outside of the volume. In the Elekta Icon case this would give doubling of the data size, double volume size and some reduced accuracy due to the resampling. Perhaps we can live with that?

adler-j commented 8 years ago

I now consider this solved since it can be done by fourier transform and we have an example of it. Long run, we could perhaps create a proper function for it, but currently it is about 4 extra lines.