search

JuliaImageRecon / ImagePhantoms.jl

Software phantoms for image reconstruction
MIT License
11 stars 4 forks source link

Add cone #62

Closed JeffFessler closed 2 years ago

JeffFessler commented 2 years ago

Will address #57 for phantom, leaving radon and spectrum for future work.

JeffFessler commented 2 years ago

@tknopp this version puts the base at the origin with the cone pointing up along z. If you prefer the vertex at the origin just let me know - it is a trivial change.

codecov[bot] commented 2 years ago

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage :thumbsup:

Coverage data is based on head (734ac01) compared to base (419661e). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #62 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 19 20 +1 Lines 581 592 +11 ========================================= + Hits 581 592 +11 ``` | [Impacted Files](https://codecov.io/gh/JuliaImageRecon/ImagePhantoms.jl/pull/62?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [src/cone.jl](https://codecov.io/gh/JuliaImageRecon/ImagePhantoms.jl/pull/62/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2NvbmUuamw=) | `100.00% <100.00%> (ø)` | | 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 2 years ago

Base sounds fine. One could also use the center of the line going through the tip and the base (center). Or the mass center. That would even be consistent with the ellipsoid / cube, if I see it right.

JeffFessler commented 2 years ago

One could also use the center of the line going through the tip and the base (center). Or the mass center. That would even be consistent with the ellipsoid / cube, if I see it right.

I didn't quite follow; are you expressing a preference, or just mentioning options? For some shapes, like cuboid and rect, the width is the full width, whereas for others, like ellipsoid and ellipse, the width is the radii. I used whatever seemed natural for each shape, rather than striving for consistency. Probably the closest to the Cone here is the Cylinder where width[1:2] are the radii of the base and width[3] is the height. In terms of height, I think I'd prefer to match Cone and Cylinder unless you have a strong preference otherwise.

Oh wait, perhaps you are talking about where to center it along z, rather than the aspect ratio. In that case I would be fine with shifting it so that the span along z is [-0.5,0.5] instead of the current [0,1]. That shift would be consistent with the Cylinder that also spans [-0.5,0.5]. (I'd rather not use center of mass.)

OTOH, my Triangle has its base along the x-axis, rather than being centered, and I'd rather not break that. So the Cone will end up being "consistent" with only one of Cylinder and Triangle and inconsistent with the other. Which z-positioning do you prefer?

tknopp commented 2 years ago

I just wanted to express options. And yes I ment the z-axis. Whether to be now consistent with triangle or cylinder ist a tough question. I tend towards the triangle but don't have a strong opinion.

JeffFessler commented 2 years ago

tend towards the triangle

Oh good, because then no changes are needed :) I will merge after CI passes. Will register after I am sure the cone docs look ok.