odlgroup / odl

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

ASTRA scaling factor change in next release #1415

Open wjp opened 6 years ago

wjp commented 6 years ago

Hi,

A quick heads-up: in the next ASTRA release the scaling factor of the ASTRA BP code ODL uses has been changed by a factor reco_space.cell_volume ** 2 (since commit astra-toolbox/astra-toolbox@9fa2ffdc5348f8f19de48d06a72b82bdc1ba8f22 ). This should only require changing

scaling_factor *= (src_radius ** 2 * det_px_area ** 2 / reco_space.cell_volume ** 2)

into

scaling_factor *= (src_radius ** 2 * det_px_area ** 2)

in astra_cuda_bp_scaling_factor for ConeFlatGeometry.

adler-j commented 6 years ago

Just make sure we have a sharp version number so we can retain backwards compatibility. Should be an easy fix!

ozanoktem commented 6 years ago

Somewhat off-topic, but not entirely: @wjp , what happened to the initiative for introducing native support for curved detectors (cylindrical and spherical) in ASTRA? Just curious.

PS: I am aware that this post is better suited as a feature request at ASTRA Git repo.

wjp commented 5 years ago

One more heads up:

I've done a complete overhaul of all 2D and 3D operator scaling, to get rid of some lingering inconsistencies in definition differences between 2D/3D and parallel/fan beam geometries, in particular for detector pixel scaling and oblique projections.

In the upcoming version, all projectors scale their output to match mathematical line integrals, regardless of detector pixel size, obliqueness, etc. (I assume ODL has realized this was the way to go a long time ago :-) )

Furthermore, I've made the approximation of the GPU cone beam BP density weighting more accurate (at hopefully limited computational cost since the extra factor could actually be folded into existing constants, it turns out), and also adopted the 2D fan beam GPU code to match. This density weighting is now enabled by default.

On top of this, the annoying distance scaling factor you had to use to correct the DensityWeighting in ODL is now handled automatically. The remaining adjoint scaling ODL has to do should now be limited to the different inner products on the ODL spaces vs the ASTRA spaces.

And finally, these changes are detectable by doing astra.has_feature('projectors_scaled_as_line_integrals') and astra.has_feature('fan_cone_BP_density_weighting_by_default')

The code is currently at https://github.com/wjp/astra-toolbox/tree/consistent_scaling .

Once this branch is merged (and the remaining Windows build issues are fixed...), we should be about ready for a 2.0.0 pre-release.

kohr-h commented 5 years ago

Wow, that sounds like a huge improvement to me! Thanks for putting in this work @wjp! That means we can finally freeze our hacky weighting correction function.

adler-j commented 5 years ago

Perhaps we should strive for a nice transition here. Would you have time to look into the branch @kohr-h?

kohr-h commented 5 years ago

I'll try :-)

wjp commented 5 years ago

Let me know if there's anything I can do to help. (E.g., bump the version on the branch, build a conda package, ...)