Closed kczimm closed 4 years ago
Would be great to have!
There's a randon
method here: https://discourse.julialang.org/t/radon-and-iradon-transforms/2407/11?u=tim.holy, you might compare your own implementations to it. Would need tests and iradon
, of course.
Where should we put it? Perhaps we need an ImageReconstruction.jl package?
Perhaps we need an ImageReconstruction.jl package?
Sure, and a good place for MRI related codes
I love it. I’ll start it and hopefully I can get some periodic feedback from you all.
@timholy it looks like the implementation from discourse was a ray-driven method whereas I implemented a pixel-driven method. It would be nice to have both, I think. I’m not sure how we would present the API but that can be worked out.
We've chosen algorithms by a variety of methods; for example, ImageFiltering has ways to choose FIR or FFT. But a lot of that API was before Julia's keyword-arguments were mature, so I'm not sure I'd do it that way now. If the output type is independent of the algorithm, I think a keyword switch would be just fine.
I just set up ImageReconstruction.jl and invited you, @kczimm.
Another set of API choice I and @zygmuntszpak being favor of is to wrap algorithm into a struct (with or without fields), and write implementations in its functor call, for example,
The idea is to first implement each algorithm independently and layer them up with a nice and meaningful name (e.g., reconstruct
) and better user experience, e.g., smart algorithm choice based on image content.
IMO this design is friendly to extensions, a detailed explanation related to this can be found at https://nextjournal.com/johnnychen94/the-principles-of-imagesjl-part-i
I'm closing this as it's no longer an issue now.
I'm wondering if @JeffFessler is interested in introducing his MIRT into JuliaImages
Yes, I am interested. I could work on it after the semester ends. What would that entail? MIRT.jl already has the equivalent of "radon" for parametric objects (namely ellipses and rectangles) for making sinograms (parallel beam and fan beam) of phantoms like the Shepp-Logan phantom. But I have not yet ported FBP from matlab to Julia.
If the final goal is to reexport MIRT from Images.jl...
I noticed the codebase has a lot of MATLAB-style legacies (e.g., https://github.com/JeffFessler/MIRT.jl/blob/master/src/mri/mri_objects.jl#L494-L503), so it might be reviewed and perhaps partially rewrote accordingly in the Julia style.
MIRT seems to contain codes including IO, algorithm, and display. Not very sure how codes are developed, but for JuliaImages, it's the algorithm part that's needed.
This is a long-term goal, perhaps a fresh start on ImageReconstruction would be a better strategy since it doesn't break the current API of MIRT. If we choose to do so, perhaps you could watch that package and kindly review some of the PRs there since you're the expert.
FYI, the general naming and API guideline can be found in https://github.com/JuliaImages/Images.jl/issues/767 and ImageContrastAdjustment is one that sticks to this guideline.
cc: @timholy
@JeffFessler, it would be great to collaborate! MIRT seems so big that we probably don't want to re-export it from Images.jl, but having it well-integrated into the JuliaImages ecosystem would be a huge win. Let us know what kind of support you might need.
@timholy, I chuckled that you called MIRT "so big" because the Julia version currently is about 11k lines where as my Matlab version is over 130k lines. So the 11k is just the tip of the iceberg :) (I've been wondering if I should break it down into multiple smaller packages...) Anyway, maybe you can point me to something that is "well integrated into the JuliaImages ecosystem" as an example that I can study?
It is big because of the large dependency. Unlike the commercial product MATLAB, in the "free" open-source world, a large dependency chain across multiple organizations usually means a hard-to-maintain package; for many reasons, people may stop development and it's also likely to get lost when resolving the compatibility. Breaking it down into multiple smaller packages have real merits for MIRT to integrate with other packages, and usually, you'll find many glue codes are unnecessary if they're organized well.
Take ImageView
as an example, this heavy package is split out from the core functionality of Images ecosystem so that whoever uses Images.jl
don't get annoyed by the tens of binary dependencies, especially when some of them are not under active maintenance (e.g., Gtk.jl
). This is also why it is hard to diagnose ImageView when it errors. Whoever wants to display an image can choose the one (s)he likes from Plots, ImageShow, Makie, ImageInTerminal and ImageView.
Another advantage of breaking down into smaller packages is that people outside your lab can more easily understand the codes they're interested in, and can help maintain it.
There are some disagreements, but I personally read ImageQualityIndexes and ImageContrastAdjustment the future of how individual algorithms are developed and organized in a julia way. They're purely numerical and algorithmic and don't deal with image IO and visualization.
I think the core idea of "well integrated into the JuliaImages ecosystem" is composability. I'm not sure if this is the case of MIRT but let me make an example. If an image wrapped by a struct that cannot be treated as an array, or if an algorithm doesn't accept a generic array as input, it's a smell of bad comparability since people have to write many glue codes to make it work.
the Julia version currently is about 11k lines where as my Matlab version is over 130k lines
Hah! How much of the functionality of the Matlab version is present in the Julia version? I.e., is the Julia ecosystem wildly efficient or is it mostly just a difference in comprehensiveness?
What I would define as a "big" dependency is one that would substantially increase the time needed for using Images
:
julia> @time using Images
7.952959 seconds (13.38 M allocations: 705.574 MiB, 3.06% gc time)
julia> @time using MIRT
12.892684 seconds (24.60 M allocations: 1.193 GiB, 3.33% gc time)
We've kept some significantly smaller packages out (though we could reconsider that),
julia> @time using Images
7.922947 seconds (13.38 M allocations: 705.578 MiB, 3.23% gc time)
julia> @time using ImageSegmentation
0.647100 seconds (959.22 k allocations: 49.431 MiB, 3.16% gc time)
just to keep Images.jl itself from growing too big.
With respect to package organization...I would agree with basically everything @johnnychen94 said, though I would also admit that there are some pain points to splitting things up. For example, we are currently thinking about moving a few method definitions between ColorVectorSpace and ColorTypes, and in addition to the raw code changes (which are largely copy/paste) we will have to do a certain amount of release management to ensure that the upgrade goes smoothly for everyone.
There are rumors that eventually it may be possible to keep multiple Julia packages in a single git repository, but I don't have a timeline for that and it's not an area that I myself am contributing to.
I should also say that I haven't yet looked at MIRT with any seriousness. It may already be perfect and not need a line changed. Either way, it looks like a serious contribution and I very much look forward to exploring it!
Thanks for the pointers. I'll look at this more after finals are graded. (To answer your question, I've probably ported <10% of the Matlab code to Julia so far.) I suspect that MIRT's dependence on Plots is a significant factor in its slow startup. I'm not ready to untangle that though because having test/demo routines co-located with the algorithms seems quite convenient and helpful for end users. If Julia came with a precompiled plot library (even if basic) it would sure help...
If Julia came with a precompiled plot library (even if basic) it would sure help...
As PackageCompiler matures this will become easier to arrange. I think the plotting libraries are on a steeper trajectory of changes, so the ~6-month release cycle of Julia itself is not ideal.
I was looking at the API comparison and noticed the lack of
radon
andiradon
transform implementations. Does JuliaImages want implementations of these functions? If so, what repository should a PR go to? I am happy to implement them. They would be similar to functions in a package I developed when going through Kak and Slaney's book for my own interest.