JuliaImageRecon / Sinograms.jl

Julia library for working with sinograms / tomography / Radon transform
MIT License
15 stars 7 forks source link

Use helpers instead of getproperty #47

Closed JeffFessler closed 1 year ago

JeffFessler commented 1 year ago

Huge overhaul here to improve type stability by eliminating getproperty, cf https://github.com/JuliaLang/julia/pull/48004

Instead of rg.ar to get the angles in radians, now there are helper functions like _ar(rg). (For the angles in degrees, still use angles(rg).) The many helper functions are mainly for developers so they are not exported. If they are needed by non-developers then they should get better names before exporting.

A major breaking change here is that fbp just returns the image now, instead of also the filtered sinogram, because that is the usual use. One can easily compute the sinogram if needed using fbp_sino_filter, as illustrated in one of the docs.

codecov[bot] commented 1 year ago

Codecov Report

Base: 99.63% // Head: 99.61% // Decreases project coverage by -0.01% :warning:

Coverage data is based on head (2bc70ee) compared to base (65fafe8). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #47 +/- ## ========================================== - Coverage 99.63% 99.61% -0.02% ========================================== Files 31 31 Lines 822 789 -33 ========================================== - Hits 819 786 -33 Misses 3 3 ``` | [Impacted Files](https://codecov.io/gh/JuliaImageRecon/Sinograms.jl/pull/47?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [src/Sinograms.jl](https://codecov.io/gh/JuliaImageRecon/Sinograms.jl/pull/47/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL1Npbm9ncmFtcy5qbA==) | `100.00% <ø> (ø)` | | | [src/fbp/d-angle.jl](https://codecov.io/gh/JuliaImageRecon/Sinograms.jl/pull/47/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2ZicC9kLWFuZ2xlLmps) | `100.00% <100.00%> (ø)` | | | [src/fbp/filter.jl](https://codecov.io/gh/JuliaImageRecon/Sinograms.jl/pull/47/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2ZicC9maWx0ZXIuamw=) | `100.00% <100.00%> (ø)` | | | [src/fbp/parker.jl](https://codecov.io/gh/JuliaImageRecon/Sinograms.jl/pull/47/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2ZicC9wYXJrZXIuamw=) | `100.00% <100.00%> (ø)` | | | [src/fbp/ramp.jl](https://codecov.io/gh/JuliaImageRecon/Sinograms.jl/pull/47/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2ZicC9yYW1wLmps) | `100.00% <100.00%> (ø)` | | | [src/fbp/window.jl](https://codecov.io/gh/JuliaImageRecon/Sinograms.jl/pull/47/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2ZicC93aW5kb3cuamw=) | `100.00% <100.00%> (ø)` | | | [src/fbp2/back-fan.jl](https://codecov.io/gh/JuliaImageRecon/Sinograms.jl/pull/47/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2ZicDIvYmFjay1mYW4uamw=) | `97.87% <100.00%> (-0.09%)` | :arrow_down: | | [src/fbp2/back-par.jl](https://codecov.io/gh/JuliaImageRecon/Sinograms.jl/pull/47/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2ZicDIvYmFjay1wYXIuamw=) | `97.22% <100.00%> (-0.15%)` | :arrow_down: | | [src/fbp2/fbp-par.jl](https://codecov.io/gh/JuliaImageRecon/Sinograms.jl/pull/47/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2ZicDIvZmJwLXBhci5qbA==) | `100.00% <100.00%> (ø)` | | | [src/fbp2/fbp.jl](https://codecov.io/gh/JuliaImageRecon/Sinograms.jl/pull/47/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2ZicDIvZmJwLmps) | `100.00% <100.00%> (ø)` | | | ... and [19 more](https://codecov.io/gh/JuliaImageRecon/Sinograms.jl/pull/47/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

tknopp commented 1 year ago

@JeffFessler

I just tried it out and now get an error:

DomainError with -Inf:

sin(x) is only defined for finite x.

sin_domain_error(::Float64)@trig.jl:28
sin(::Float64)@trig.jl:39
#34@parker.jl:88[inlined]
_broadcast_getindex_evalf@broadcast.jl:670[inlined]
_broadcast_getindex@broadcast.jl:643[inlined]
getindex@broadcast.jl:597[inlined]
macro expansion@broadcast.jl:961[inlined]
macro expansion@simdloop.jl:77[inlined]
copyto!@broadcast.jl:960[inlined]
copyto!@broadcast.jl:913[inlined]
materialize!@broadcast.jl:871[inlined]
materialize!@broadcast.jl:868[inlined]
parker_weight_fan_short(::Int64, ::Int64, ::Float64, ::Float64, ::StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}, ::Vector{Float64}, ::Float64, ::Type{Float32})@parker.jl:98
parker_weight_fan_short@parker.jl:73[inlined]
parker_weight(::Sinograms.SinoFanFlat{Unitful.Quantity{Float64, 𝐋, Unitful.FreeUnits{(cm,), 𝐋, nothing}}, Float64}, ::Type{Float32})@parker.jl:118
parker_weight@parker.jl:115[inlined]
_fbp_weights(::Sinograms.SinoFanFlat{Unitful.Quantity{Float64, 𝐋, Unitful.FreeUnits{(cm,), 𝐋, nothing}}, Float64})@plan2.jl:142
#plan_fbp#53@plan2.jl:119[inlined]
reco(::ImageMetadata.ImageMeta{Float32, 2, Matrix{Float32}, Dict{Symbol, Any}}, ::Tuple{Int64, Int64}, ::Tuple{Unitful.Quantity{Float64, 𝐋, Unitful.FreeUnits{(cm,), 𝐋, nothing}}, Unitful.Quantity{Float64, 𝐋, Unitful.FreeUnits{(cm,), 𝐋, nothing}}}, ::Float64)@[Other: 34](http://localhost:1234/edit?id=315ae8ce-8aa2-11ed-1cdb-d15746ce3525#)
top-level scope@[Local: 13](http://localhost:1234/edit?id=315ae8ce-8aa2-11ed-1cdb-d15746ce3525#)

Apparently there is a division by zero in this line: https://github.com/JuliaImageRecon/Sinograms.jl/blob/main/src/fbp/parker.jl#L97

Not sure how to debug this further.