search

LArbys / LArCV

Liquid Argon Computer Vision
11 stars 9 forks source link

When a channel is selected in one of the RGB menus, change plot without undoing changes from OpenCV #25

Closed twongjirad closed 8 years ago

twongjirad commented 8 years ago

This way, I can erase data from all channels.

Right now, I have to hit Replot to change the viewed channels. But this reloads the image from disk and undoes any modifications.

twongjirad commented 8 years ago

Proposed solution is here: https://github.com/LArbys/LArCV/commit/33207d2c7d925cac1c1cbead8782d66f316d6d01

This requires a redesign of the data interface layer. Right now we have orig_mat and plot_mat. The latter is thresholded for viewing contrast only. orig_mat on the other hand represents a working copy of the images that can get modified by cv and then passed to the network.

However, orig_mat and plot_mat are only 3 channels. For more channels (like the 12 channel data), we want to be able to swap the channels stored in both orig_mat and plot_mat while retaining the changes in orig_mat.

So here I added (in ch12image.py) another copy of the images: work_mat. This has all 12 channels. When channels are swapped, the contents of orig_mat are put into work_mat, and then the values of the new channels to view from work_mat are put into orig_mat.

This is big change in structure. So I will initiated a pull request that @vgenty should review. Maybe he will have a better solution more inline with his vision.

twongjirad commented 8 years ago

As promised: https://github.com/LArbys/LArCV/pull/26

vgenty commented 8 years ago

Thanks for this fix, I don't have an issue with this method thanks for implementing. I didn't anticipate 12 channel image at all and I'm kicking myself in the butt during this whole process. The intermediate swapping matrix is necessary since with OpenCv the current set up and only operate on 3 channels at a time, swapping in and out is a fantastic addition. One more thing -- I notice that plotData had "refresh" as an input and it didn't look like it was being used so I removed, I hope this is OK

vgenty commented 8 years ago

I will go in and make sure the image types -- false color and defaultimage follow the same procedure. Although as Kazu probably already mentioned it's probably better to have a standard way to load the image eliminating the need for someone to subclass PlotImage. I thought it was a good idea at the time but maybe blew up in my face when caffe interface came along. Growing pains!

vgenty commented 8 years ago

forgot the commit number for plotData refresh: https://github.com/LArbys/LArCV/commit/57a13d14917da09527094501d8ff48f708e42620

twongjirad commented 8 years ago

Yes. Thanks for noticing the useless refresh variables! They were from a previous attempt. Then I decided to go with a new function (swap_...) entirely.