-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
radon/iradon functions #113
Comments
Would be great to have! There's a Where should we put it? 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., 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? |
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 :) |
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 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. |
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 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. |
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.The text was updated successfully, but these errors were encountered: