JuliaImages / ImageReconstruction.jl

MIT License
8 stars 6 forks source link

Inverse radon #2

Closed kczimm closed 4 years ago

kczimm commented 4 years ago

A pixel-driven implementation of the inverse radon transform.

timholy commented 4 years ago

Have you benchmarked against the version I posted in discourse? I can do that if you haven't, but perhaps you already know yours is faster?

kczimm commented 4 years ago

No, I haven't benchmarked it yet. I actually don't know how they will compare. I'm not even confident that my implementation is optimized but it works. I need to add some tests. How do you feel about the test being a threshold on the cross entropy between the two images?

johnnychen94 commented 4 years ago

weird that CI doesn't get triggered 😕

How do you feel about the test being a threshold on the cross entropy between the two images?

wrt tests, perhaps ReferenceTests can be used here. It's not documented yet but it supports custom success_or_fail function, e.g.,

using ImageQualityIndexes

@test_reference "ref.png" img by=(actual,ref)->psnr(actual,ref)>30
kczimm commented 4 years ago

@timholy your ray-based implementation on discourse is substantially faster.

julia> @benchmark ImageReconstruction.radon($I, $θ, $t)
BenchmarkTools.Trial: 
  memory estimate:  470.39 KiB
  allocs estimate:  2
  --------------
  minimum time:     150.499 ms (0.00% GC)
  median time:      168.415 ms (0.00% GC)
  mean time:        184.142 ms (0.00% GC)
  maximum time:     431.614 ms (0.00% GC)
  --------------
  samples:          28
  evals/sample:     1
julia> @benchmark ImageReconstruction.radon_discourse($I, $θ, $t)
BenchmarkTools.Trial: 
  memory estimate:  471.92 KiB
  allocs estimate:  18
  --------------
  minimum time:     18.155 ms (0.00% GC)
  median time:      18.654 ms (0.00% GC)
  mean time:        19.014 ms (0.23% GC)
  maximum time:     30.507 ms (38.27% GC)
  --------------
  samples:          263
  evals/sample:     1

I did implement your threaded version, to be fair, but it wasn't close before that. I am only using 2 threads.

timholy commented 4 years ago

Cool. I have no objections to you putting it in this package; it's my own work and I hereby :smile: license it under MIT.