lucasb-eyer / pydensecrf

Python wrapper to Philipp Krähenbühl's dense (fully connected) CRFs with gaussian edge potentials.
MIT License
1.95k stars 417 forks source link

Problem with non-RGB input image #26

Closed DaveRichmond- closed 7 years ago

DaveRichmond- commented 7 years ago

Hi Lucas,

I recently started using your wrapper for Phillip Krahenbuhl's fully connected CRFs. Very useful -- thanks for making this available!

I'm working through a simple example, to make sure that I understand the usage, and I came across something a bit strange. If I input an image that doesn't have 3 channels (eg, it could be grayscale, or a feature map with more than 3 channels), I get a nonsense output. I understand that typical usage would be for RGB images, but is there a way to use images without exactly 3 channels?

Here is a gist with an example: https://gist.github.com/DaveRichmond-/3d4b45f06a1fedb2527dae3a0856b213

Thanks for your help.

Best, Dave

lucasb-eyer commented 7 years ago

Thank you for the gist, this will make investigation easier. I'm looking into it as a number different than 3 channels should definitely work.

lucasb-eyer commented 7 years ago

OK, I figured it out. It's not a bug in the library per se, but the library also doesn't protect you from a mistake and just computes garbage. I added some checks to protect from this mistake in this commit as well as an explanation in the readme, go check it out.

With this change, your example code should now raise a ValueError instead of computing wrong results. What you need to do to get it working correctly is replace your call to d.addPairwiseBilateral with the following:

energy = create_pairwise_bilateral(sdims=(10,10), schan=0.01, img=tmp_img, chdim=2)
d.addPairwiseEnergy(energy, compat=10)

and it will work.

I do like your example code, and it would be great to have some example code for non-RGB data. Would you be fine having your example as part of PyDenseCRF? If yes, I recommend you open a pull-request adding the file to the examples/ subfolder so that you get the credit you deserve (I can do minor cleanup and add comments thereafter), but if you don't have the time for that and if you allow it, I can add it myself too.

For reference, here is the image from "before" with the mistake, and then the image from "after" using the above fix:

issue26_problem

issue26_solution

DaveRichmond- commented 7 years ago

Hi Lucas,

Thanks for your email. That makes sense -- I was able to run the code with 'createPairwiseBilateral' and 'addPairwiseEnergy' but it's quite slow to set up the graph, so I was trying to get 'addPairwiseBilateral' to work. If I have time, I may also try to speed up createPairwiseBilateral and do a pull request.

Yes, it would be fine to include my example in your repo. I will make the changes that you suggested, so that it's working properly, and then make a pull request.

Thanks again for your help!

Best, Dave

On Sat, Feb 11, 2017 at 9:07 AM, Lucas Beyer notifications@github.com wrote:

OK, I figured it out. It's not a bug in the library per se, but the library also doesn't protect you from a mistake and just computes garbage. I added some checks to protect from this mistake in this commit https://github.com/lucasb-eyer/pydensecrf/commit/3381ed438f7c7a4ce86998c045d31897bee5fa07 as well as an explanation in the readme, go check it out.

With this change, your example code should now raise a ValueError instead of computing wrong results. What you need to do to get it working correctly is replace your call to d.addPairwiseBilateral with the following:

energy = create_pairwise_bilateral(sdims=(10,10), schan=0.01, img=tmp_img, chdim=2) d.addPairwiseEnergy(energy, compat=10)

and it will work.

I do like your example code, and it would be great to have some example code for non-RGB data. Would you be fine having your example as part of PyDenseCRF? If yes, I recommend you open a pull-request adding the file to the examples/ subfolder so that you get the credit you deserve (I can do minor cleanup and add comments thereafter), but if you don't have the time for that and if you allow it, I can add it myself too.

For reference, here is the image from "before" with the mistake, and then the image from "after" using the above fix:

[image: issue26_problem] https://cloud.githubusercontent.com/assets/1476029/22854284/3e1d8a3a-f06b-11e6-8b1e-b2e44904bb1e.png

[image: issue26_solution] https://cloud.githubusercontent.com/assets/1476029/22854285/4393cf60-f06b-11e6-887d-7914608b2ef2.png

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/lucasb-eyer/pydensecrf/issues/26#issuecomment-279145916, or mute the thread https://github.com/notifications/unsubscribe-auth/AIBouq_Y3nvZ-QTCSD7XlTTCxiRrKCQYks5rbcCagaJpZM4L7aq3 .

DaveRichmond- commented 7 years ago

Hi Lucas,

I just made the PR. I only committed the notebook because this is all I have played around with. If you prefer to have .py format, there is a simple conversion tool (nbconvert). I made some comments, but please feel free to add more. And of course, let me know if you need anything else.

Thanks, Dave

On Mon, Feb 13, 2017 at 1:09 PM, David Richmond daverichmond@gmail.com wrote:

Hi Lucas,

Thanks for your email. That makes sense -- I was able to run the code with 'createPairwiseBilateral' and 'addPairwiseEnergy' but it's quite slow to set up the graph, so I was trying to get 'addPairwiseBilateral' to work. If I have time, I may also try to speed up createPairwiseBilateral and do a pull request.

Yes, it would be fine to include my example in your repo. I will make the changes that you suggested, so that it's working properly, and then make a pull request.

Thanks again for your help!

Best, Dave

On Sat, Feb 11, 2017 at 9:07 AM, Lucas Beyer notifications@github.com wrote:

OK, I figured it out. It's not a bug in the library per se, but the library also doesn't protect you from a mistake and just computes garbage. I added some checks to protect from this mistake in this commit https://github.com/lucasb-eyer/pydensecrf/commit/3381ed438f7c7a4ce86998c045d31897bee5fa07 as well as an explanation in the readme, go check it out.

With this change, your example code should now raise a ValueError instead of computing wrong results. What you need to do to get it working correctly is replace your call to d.addPairwiseBilateral with the following:

energy = create_pairwise_bilateral(sdims=(10,10), schan=0.01, img=tmp_img, chdim=2) d.addPairwiseEnergy(energy, compat=10)

and it will work.

I do like your example code, and it would be great to have some example code for non-RGB data. Would you be fine having your example as part of PyDenseCRF? If yes, I recommend you open a pull-request adding the file to the examples/ subfolder so that you get the credit you deserve (I can do minor cleanup and add comments thereafter), but if you don't have the time for that and if you allow it, I can add it myself too.

For reference, here is the image from "before" with the mistake, and then the image from "after" using the above fix:

[image: issue26_problem] https://cloud.githubusercontent.com/assets/1476029/22854284/3e1d8a3a-f06b-11e6-8b1e-b2e44904bb1e.png

[image: issue26_solution] https://cloud.githubusercontent.com/assets/1476029/22854285/4393cf60-f06b-11e6-887d-7914608b2ef2.png

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/lucasb-eyer/pydensecrf/issues/26#issuecomment-279145916, or mute the thread https://github.com/notifications/unsubscribe-auth/AIBouq_Y3nvZ-QTCSD7XlTTCxiRrKCQYks5rbcCagaJpZM4L7aq3 .

lucasb-eyer commented 7 years ago

A PR speeding up createPairwiseBilateral would be very welcome when you get around to it; I never used it for much data, so didn't feel its speed (or lack thereof). Since you are using notebooks, line_profiler will come in very handy for finding the bottleneck! %load_ext line_profiler to load it and then %lprun -f function_name function_name(args) to profile.

Finally, just in case you didn't realize, I didn't send you an e-mail, but I answered in the issue on GitHub and you got an automatic notification e-mail about it. GitHub is nice in that it can handle replies to such e-mails. You (and anybody) can follow this discussion here on Github.

Thanks for everything!