Skip to content
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

Adding radon transform #705

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

tosalonijain
Copy link

I have tried building a rough code for radon transform and am expecting guidance and changes from your side to finish it.

@codecov
Copy link

codecov bot commented Mar 10, 2018

Codecov Report

Merging #705 into master will decrease coverage by 2.9%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #705      +/-   ##
==========================================
- Coverage   89.18%   86.27%   -2.91%     
==========================================
  Files          13       14       +1     
  Lines        1211     1290      +79     
==========================================
+ Hits         1080     1113      +33     
- Misses        131      177      +46
Impacted Files Coverage Δ
src/radon.jl 0% <0%> (ø)
src/showmime.jl 100% <0%> (ø) ⬆️
src/Images.jl 91.66% <0%> (ø) ⬆️
src/algorithms.jl 86.58% <0%> (+1.1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 59750c3...76e5936. Read the comment docs.

@Evizero
Copy link
Member

Evizero commented Mar 11, 2018

Hi! Thank you for your interest to contribute!

Did you create this according to some reference? I am asking because I lack the domain knowledge to put this into context. From a pure package-maintainer standpoint I will say that I don't think its a good idea to introduce a couple new dependencies for a single function (especially ImageView).

@zygmuntszpak
Copy link
Member

zygmuntszpak commented Mar 11, 2018

Have a look at Algorithm 2.1 'The Discrete Normal Radon Transform` page 28 (47 in PDF) in
the PhD Thesis of Peter Toft. I would be expecting something along those lines...

@tosalonijain
Copy link
Author

tosalonijain commented Mar 15, 2018

@Evizero @zygmuntszpak , yes I have used the scipy radon transform as a reference to frame this code. I am sorry I involved too many dependencies, I would cut the unnecessary ones.
I had some queries to give this code the final finishing. It would be great if I could get some guidance from your side..:)
And I am having trouble in doing a simple projective transformation of the image. I wanted to know if
https://github.com/peterkovesi/ImageProjectiveGeometry.jl/blob/master/doc/projective.md#imgtrans
this function could help me doing the same.
I know the code has too many imperfections. But I seek your guidance to give it a finishing touch asap because I have already written an inverse radon code and there's no meaning of adding it without having the radon transform feature available in the packages.

@tosalonijain
Copy link
Author

@Evizero @zygmuntszpak

@zygmuntszpak
Copy link
Member

Could you please outline the steps in the algorithm that you are trying to implement? Its not clear to me why you would need a projective transformation. The only explanation I can come up with for what you might be trying to do is that you are summing the rows for each column of the image, rotating the image slightly, summing the rows for each column of the rotated image etc. If that is the case, you don't need a projective warp but just a simple rotation. Presumably when you rotate the image slightly the new coordinates of the pixels don't fall onto the pixel grid, and so you need to fill the pixel grid of the rotated image by interpolating?

The algorithm that I referenced in my previous post follows a different approach and seems like it can be implemented almost verbatim in Julia. Why not give that one a go?

image

@tosalonijain
Copy link
Author

@zygmuntszpak , I am sorry I was just trying a different approach to find the radon transform. I was following the code base of scipy radon transform.
Here, I have implemented a similar approach as suggested by you. Please provide your views and required changes, documentation etc for this code.
If it seems okay to you, I shall soon write a test code for radon transform.
And it would be great if you could provide some similar sources to apply inverse radon over the radon image produced by it.
Thankyou.

@zygmuntszpak
Copy link
Member

Chapter 8 of the thesis I referenced deals with inverting the radon transform using the filtered back-projection technique. The procedure is split across 3 algorithms (8.1, 8.2 and 8.3). The implementation of the inverse radon transforms appears to be substantially more involved than the radon transform.

@zygmuntszpak
Copy link
Member

I was wondering why you implemented something that was similar to the algorithm outlined above but not identical. I see that you are actually using Tim Holy's implementation...

@tosalonijain
Copy link
Author

tosalonijain commented Mar 17, 2018

@zygmuntszpak actually earlier only Tim had referenced me that implementation. But I was interested to employee interpolations on both x and y. Then finally since I could not build a feasible code for that so, I decided to use this only. I shall be writing the iradon code, as per your suggestions. Beside that the documentation and test for radon needs to be done also. Seeking your guidance.
Thankyou.

@zygmuntszpak
Copy link
Member

So far I haven't used the Radon transform in my own work so I don't have a deep understanding of it. Hence it would take me a fair bit of time to discern how Tim's implementation relates to the Algorithm that I cited above. Perhaps you could add some comments to clarify the steps of the algorithm?

If I were doing this for the first time, I would start by implementing something where the key ideas are explained and the pseudo-code is detailed such as the Algorithm I pasted above. Then, upon receiving Tim's implementation, I would first check that both implementations give my roughly the same results. If they do, I would start teasing apart Tim's implementation to see why it is faster etc.

There are some things that I personally find a bit odd in Tim's implementation. For example, instead of w, h = size(A) I would have thought that h, w = size(A) would be more appropriate since the usual convention is that rows of a matrix determine the height and the columns determine the width.

Perhaps someone else might have better insight into why w,h was chosen instead. The discourse thread I referenced above also has an implementation of the inverse radon transform which you might be able to use as a basis.

Also, it might be worth checking out the ImageJ implementation of the radon transforms. Since ImageJ is used heavily by the medical imaging community I'm pretty sure a lot of thought went into the optimality and correctness of the implementation.

@tosalonijain
Copy link
Author

@zygmuntszpak I have added comments in order to help explanation of the code.
There's slight variation from the reference you provided because the pseudo code mentioned in the thesis works for radon transform assuming the image to be square whereas Tim's code deals with rectangular images also. Although the governing idea is same for both.
I am waiting for the changes to be made according to you and also shall I start writing the test code for this? I would work hard to get this PR get merged asap.
Thankyou.

@zygmuntszpak
Copy link
Member

Thank you, Saloni. I reckon you may as well start working on writing the tests for this.

@tosalonijain
Copy link
Author

tosalonijain commented Mar 20, 2018

@zygmuntszpak I am having trouble in how to claim for a given matrix or image, the radon transform found is correct without employing the inverse radon on that produced image?
It would be great if you could offer few suggestions in this context for writing the test code.
Also I want to apply for GSoC 2k18 in JuliaImages. I am considering the project : Integration of OpenCV and JuliaImages. Mentors : Maximiliano Suster and Tim Holy.
But since I could not manage any communication with the mentors as they seem to be very busy, I wanted to know if you could help me with that?
Thank you.

@zygmuntszpak
Copy link
Member

You can test the radon transform on a simple shape for which an analytic expression of the radon transform is known. For example, you could consider a unit disk (filled circle) centred at the origin.
A formula for what the radon transform should produce on such shape can be found on page 2 of these lecture notes. Look at the sentence "Then an easy calculation gives...".

I'm sorry but I won't be able to assist you with the OpenCV project.

@zygmuntszpak
Copy link
Member

Thanks for adding such detailed comments to the code–they help tremendously!

@tosalonijain
Copy link
Author

@zygmuntszpak , I am sorry I am a a bit busy making my proposal for OpenCV.jl project, so could not implement the test code. But I would very soon be completing this work of mine.
Thank you.

@zygmuntszpak
Copy link
Member

Are you still busy with some of the tests?

@tosalonijain
Copy link
Author

@zygmuntszpak I am sorry I could not address your comment. Yes actually I am pretty swamped with my university tests and exams. I would for sure implement the test code soon. Pardon for the passiveness.

Thank you.

@ashwani-rathee
Copy link
Member

This is also very unlikely to be ever merged given radon transform is provided by https://github.com/JuliaImages/ImageReconstruction.jl

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants