clij / clijx

Other
4 stars 6 forks source link

add radius parameter to generateGreyValueCooccurrenceMatrixBox #15

Open haesleinhuepf opened 4 years ago

haesleinhuepf commented 4 years ago

https://forum.image.sc/t/glcm-at-different-distances/42924

Bobafotz commented 4 years ago

Hello @haesleinhuepf ! I dig into the code and I'm quite proud to say I did the modification I was asking for ! You can have a look here : https://github.com/Bobafotz/clijx_GLCM_mod I forked clijx in order to show you the end result on Github, I don't know if it is the right thing to do. I didn't know Java, neither plugin compilation, but hey, at the end, it worked ! To be fair, not completely, I can't find where you manage the GUI of the generateGreyValueCooccurrenceMatrixBox function to add the radius parameter (named pixel_distance in my code). But when i call it in my plugin, it worked. I compared the result of the modified function with what I have using the ImageJ ops (where the radius parameter exists already) and the GLCM and the Haralick value are the same in 2D and in 3D with a radius=1, but they tend to be quite different with bigger radius (see the attached file : REFERENCE_CLIJ.xlsx where I compared the two version at several distances). I don't see where does it come from yet. If I have anything new, I would let you know. Have a good day

Julien

haesleinhuepf commented 4 years ago

Hey Julien @Bobafotz ,

I'm glad to hear that you dived into this!

I can't find where you manage the GUI of the generateGreyValueCooccurrenceMatrixBox function to add the radius parameter (named pixel_distance in my code).

Well, you need to enter the parameter to the parameter-help-text. And you did that here. If that doesn't work from your point of view, I'd like to know where the issue is.

compared the result of the modified function with what I have using the ImageJ ops [...] but they tend to be quite different with bigger radius

So I can't see obvious issues with the code, but I'd suggest carfully checking all the changes. For example here it seems your change leads to not processing the whole image anymore. I'm also not an expert but this line was meant to check if the pixel is still within the current line. I'm not sure if it makes sense multiply width in this context with pixel_distance.

I was also wondering why you committed Eclipse project files? Are they necessary? It should actually be possible to import the project into Eclipse via the pom.xml file. I work with IntelliJ and also don't upload IntelliJ project files to github because I would like to make it work with any IDE. If that doesn't work with Eclipse, please let me know and I'll fix it.

Last but not least, I'd suggest renaming the "pixel_distance" to something like "radius" to prevent confusion. In imaging, pixel-size and pixel-distance are terms used to configure microscopes. The radius parameter already exists in some plugins and thus, might make things easier.

Feel free to file pull request or PR-draft. That makes discussion about code a bit easier as I can leave comments in line.

Let me know how it goes!

Cheers, Robert

Bobafotz commented 4 years ago

Hello @haesleinhuepf,

Well, you need to enter the parameter to the parameter-help-text. And you did that here. If that doesn't work from your point of view, I'd like to know where the issue is.

My bad, it works, I didn't check since the modification of the parameter-help-text.

I was also wondering why you committed Eclipse project files? Are they necessary?

No, not at all, it's just a mistake in the use of Github, I am a newbie with this as well. I removed them.

It should actually be possible to import the project into Eclipse via the pom.xml file.

And it is, I imported the pom.xml in Eclipse and succeed to make it work (by tweaking the maven-surefire-plugin version and adding a <testFailureIgnore>), so no problem on this side.

I will carefully look at the other points you mentioned and check again their relevance. Also, I will rename pixel_distance as radius or something related in order to avoid any confusion.

Thank you again

Julien