Image registration a bit buggy (appears to hold onto old registrations)

[twVolc]

I've encountered bugs with image registration a number of times when moving between data and generally doing processing. Sometimes it's been hard to keep track of what exactly I've done to recreate the error, so I've just let it slide, but think I need to document this now as I've found at least one way to easily reproduce a bug:

1. Start software, the GUI loads with default villaricca data, and defaut openCV registration
2. Load new image sequence (I load some Reventador data) - as expected, the registration is poor because it still has the Villaricca loaded.
3. There's a few bugs here

• If I click OpenCV straight away again, the folllowing error is thrown:

Exception in Tkinter callback Traceback (most recent call last): File "C:\Users\thom_\anaconda3\envs\pycam\lib\tkinter\__init__.py", line 1892, in __call__ return self.func(*args) File "C:\Users\thom_\Documents\Work\pycam_dev\PyCamPermanent\pycam\gui\figures_cam.py", line 466, in <lambda> command=lambda: self.img_reg_select(self.reg_meth)) File "C:\Users\thom_\Documents\Work\pycam_dev\PyCamPermanent\pycam\gui\figures_cam.py", line 540, in img_reg_select self.pyplis_worker.register_image(**kwargs) File "C:\Users\thom_\Documents\Work\pycam_dev\PyCamPermanent\pycam\so2_camera_processor.py", line 1147, in register_image self.img_B.img_warped = self.img_reg.register_image(self.img_A.img, self.img_B.img, **kwargs) File "C:\Users\thom_\Documents\Work\pycam_dev\PyCamPermanent\pycam\so2_camera_processor.py", line 4066, in register_image warped_B = self.cv_warp_img(img_B) File "C:\Users\thom_\Documents\Work\pycam_dev\PyCamPermanent\pycam\so2_camera_processor.py", line 4022, in cv_warp_img img_warped = cv2.warpAffine(img, self.warp_matrix_cv, (sz[1], sz[0]), cv2.error: OpenCV(4.9.0) D:\a\opencv-python\opencv-python\opencv\modules\imgproc\src\imgwarp.cpp:2757: error: (-215:Assertion failed) (M0.type() == CV_32F || M0.type() == CV_64F) && M0.rows == 2 && M0.cols == 3 in function 'cv::warpAffine'

• If I first click No Registration, then click back to OpenCV, the registration works as expected, but if I open View > More Windows > Cross-correlation plot and go to the Nadeau image, I can clearly see that this absorption image doesn't perfectly reflect the Analysis Window absorption image. There is an odd feature that is not correctly alligned - it's looks to me as if this feature is still registered as the old Villaricca registration (i.e. much higher shift upwards than the Reventador registration). See below - there is some white speckled feature in the bottom right of the Analysis Window image that is shifted upwards above the volcano flank in the Nadeau plot. This part might be more of a bug in the Nadeau plot updating rather than image registration, but there's definitely a few areas where image registration doesn't quite seem to respond as expected (others that I haven't been able to specifically recreate yet, but for instance something to do with images not correctly jumping back and forth between the different registration types in the Camera window).

I would suggest keeping this issue to document any other bugs we may find relating to this.

For context on how the warping is implemented: rather than creating a new PyplisWorker attribute called img_B_warped I decided to just give PyplisWorker.img_B a new attribute called img_warped. Perhaps things aren't correctly updated at times or there is an artifact of old warping that hasn't bee updated, for instance in other attributes later in the processing pipeline that stem from img_B_warped (e.g PyplisWorker.vign_corr_B_warped)

[twVolc]

Related to this, the SO2 image in the analysis pane doesn't update after updating image registration. This is quite a significant bug as it means you can't see changes related to image registration - could lead the user to think a registration is better or worse than it really is. Dragging the ROI on the SO2 image then updates the figure to the new correct registration, but this shouldn't be required to update the plot. I'm not sure it has always been like this so I'm guessing some of our other updates have led to this bug.

[ubdbra001]

Related to this, the SO2 image in the analysis pane doesn't update after updating image registration. This is quite a significant bug as it means you can't see changes related to image registration - could lead the user to think a registration is better or worse than it really is. Dragging the ROI on the SO2 image then updates the figure to the new correct registration, but this shouldn't be required to update the plot. I'm not sure it has always been like this so I'm guessing some of our other updates have led to this bug.

How would I go about recreating this? Something like:

• Load a new image directory manually (i.e. not through a config file)
• Update the registration
• Switch to the analysis window

[twVolc]

How would I go about recreating this? Something like:

• Load a new image directory manually (i.e. not through a config file)
• Update the registration
• Switch to the analysis window

Yep I'm pretty sure this is how I did it. I think you'll notice if you then change the ROI the analysis window will update and things will look quite different. I can't remember for certain though.

[ubdbra001]

If I click OpenCV straight away again, the following error is thrown:

Okay I've got the reason for this error.

When you first load up the GUI it will load an image registration, so self.got_cv_transform will be set to True. Because of this, if you press the "OpenCV ECC" radio button again it will run this bit of code but skip the self.cv_generate_warp_matrix() method.

https://github.com/twVolc/PyCamPermanent/blob/3b74b2bf08e826bd6e2ed12340e3e9889a511635/pycam/so2_camera_processor.py#L3104-L3110

It will still run the self.cv_warp_img() method, and this will throw the error at the below section of code, because the attribute self.warp_matrix_cv is set to False when you click the radio button:

https://github.com/twVolc/PyCamPermanent/blob/3b74b2bf08e826bd6e2ed12340e3e9889a511635/pycam/so2_camera_processor.py#L3064-L3067

https://github.com/twVolc/PyCamPermanent/blob/3b74b2bf08e826bd6e2ed12340e3e9889a511635/pycam/gui/figures_cam.py#L524-L533

I can see two potential options here:

1. Only run the command if the radio button has changed value (i.e. OpenCV -> Control Point) - Is there a need for having it re-run the registration of the already selected button is clicked again?
2. Update the logic for generating the warp_matrix so it is generated when self.warp_matrix_cv is False - _This will retain the current functionality but will recalculate the warpmatrix every time

What do you think? Any alternate suggestions?

[twVolc]

Great good spot!

It would be nice to avoid regenerating the warp matrix every time OpenCV is clicked, as it makes the GUI a little unresponsive - i.e. it could be useful to toggle between OpenCV and No registration or control point quickly, to see differences. But that being said, I think there are cases (albeit unlikely) where you might want to rerun the OpenCV registration - i.e. if you load in a new image sequence from a different volcano, or even if you just want to play around with the OpenCV tuning parameters. Perhaps the best solution is acutally to have another button that is "Run OpenCV" which runs the registration again and updates everything to the new setup? The radio button for OpenCV could be disabled until self.got_cv_transform is True, which would happen either by running OpenCV registration or by loading in a registration file (either on start-up or with the GUI option).

How does this sound? Would this be relatively straightforward to implement?

[ubdbra001]

Initial changes in draft PR #245

Just to make sure I understand what you're saying, you would like:

1. Clicking the radio buttons to toggle between existing registrations without regenerating the warp matrix
2. The addition of a run button that will generate the warp matrix and transform the image

Is that right?

Assuming it is, I have additional questions:

• For 1. do you want me to update all the optical depth plots too (i.e. the one in the analysis view)? This will flipping between the options slower, and we can have it update these plots when the run button is pressed.
• I saw that a commit from quite a while ago (2b000ce6) altered the code to fix a bug (that I don't fully understand) so that it runs process_pair() rather than load_sequence() to reload the optical depth plots, but this means you then have to reload the image sequence to update the optical flow. Would you like me to take a look at that?

[twVolc]

• Clicking the radio buttons to toggle between existing registrations without regenerating the warp matrix
• The addition of a run button that will generate the warp matrix and transform the image

The run button could just be placed within the OpenCV widget. The control point registration is run whenever the points are saved and there are a matching number of points on each plot, so as far as I understand the radio button for this only toggles between registrations anyway. OpenCV is the one that has slightly od functionality at the moment. The radio button for both would ideally be disabled until there is an existing registration for them.

• For 1. do you want me to update all the optical depth plots too (i.e. the one in the analysis view)? This will flipping between the options slower, and we can have it update these plots when the run button is pressed.

Yes we need to update these plots when we change registration. I think this happens at the moment anyway, or at least it's supposed to.

• I saw that a commit from quite a while ago (2b000ce6) altered the code to fix a bug (that I don't fully understand) so that it runs process_pair() rather than load_sequence() to reload the optical depth plots, but this means you then have to reload the image sequence to update the optical flow. Would you like me to take a look at that?

It sounds like this isn't ideal. I must have messed something up here. Really when registration shifts we would want to update everything as it could change all of the processed results. There might be a clever way of doing this that means we don't have to reload everything each time we toggle between registrations - i.e. if each array is saved as an attribute so rather than regenerating them you just switch to the other array. When re-running this would of course mean we need to regenerate things, but when just toggling between without re-running we could just grab the respective optical depth and optical flow arrays and plot them?

[ubdbra001]

Okay, I think I've got it, but just to make sure:

• No registration: should stay as is, i.e. when it is clicked all the images revert to the default positioning
• Control Points: should also stay as is, i.e. when it is clicked if there are matching numbers of points on each plot then it should run the transformation. Otherwise it should produce a message box telling the user what to do.
• OpenCV should have the different behaviour:
  • When the radio button is selected it should apply whatever transformation it currently has (?)
  • When the run button is pressed it should update the transformation

Whenever any radio button is picked the corresponding optical depth plots should all be updated too.

The radio button for both would ideally be disabled until there is an existing registration for them.

Not sure I fully understand this, do you mean for the Control Points and OpenCV options?

[ubdbra001]

It sounds like this isn't ideal. I must have messed something up here. Really when registration shifts we would want to update everything as it could change all of the processed results. There might be a clever way of doing this that means we don't have to reload everything each time we toggle between registrations - i.e. if each array is saved as an attribute so rather than regenerating them you just switch to the other array. When re-running this would of course mean we need to regenerate things, but when just toggling between without re-running we could just grab the respective optical depth and optical flow arrays and plot them?

I think part of this issue with this is that the way it is now it only loads and processes a single pair of images, and so can't update the optical flow. Whereas before it was using the load_sequence() method as so is loading the current and subsequent pair, and so can successfully produce the optical flow. I had a go at reverting the code to use the load_sequence() method and it seemed to work okay. But I don't think the bug you describe in that commit would come up with the way I'm using things, so I'm wary of just changing it to this.

I think saving the outputs for each registration (both for the images, optical depths, and optical flows) would allow faster switching in theory, but I'd have to do a bit of work looking at how to put this together in practice.

[twVolc]

Yep that all sounds correct.

Not sure I fully understand this, do you mean for the Control Points and OpenCV options?

Yes - in other words if there isn't a registration available for either then it shouldn't be possible to select that radio button (this isn't how it works at the moment but I think it makes things clearer). So if an OpenCV registration hasn't yet been generated (and one wasn't loaded in the start-up config file) the radio button shouldn't be clickable because there is no OpenCV registration. You should first have to click the Run, which generates the OpenCV registration and then makes the radio button active. I think it would make sense for that run button to switch over to OpenCV registration automatically after generating the registration too - i.e. if I currently have No registration selected, I then click Run for OpenCV it would generate the registration and then apply it and shift the radio button to having OpenCV selected automatically.

[twVolc]

I think saving the outputs for each registration (both for the images, optical depths, and optical flows) would allow faster switching in theory, but I'd have to do a bit of work looking at how to put this together in practice.

If this becomes a headache it probably isn't worth it - we could probably lose too much tim to this for relatively little gain. I think really the most important thing here is just making sure there's no bug associated with image registration so that we're confident the data are being processed and displayed correctly (i.e. the priority is just fixing this issue, the rest is just added value that maybe isn't worth the time).

[ubdbra001]

Yes - in other words if there isn't a registration available for either then it shouldn't be possible to select that radio button (this isn't how it works at the moment but I think it makes things clearer). So if an OpenCV registration hasn't yet been generated (and one wasn't loaded in the start-up config file) the radio button shouldn't be clickable because there is no OpenCV registration. You should first have to click the Run, which generates the OpenCV registration and then makes the radio button active. I think it would make sense for that run button to switch over to OpenCV registration automatically after generating the registration too - i.e. if I currently have No registration selected, I then click Run for OpenCV it would generate the registration and then apply it and shift the radio button to having OpenCV selected automatically.

Okay, in that case should the available registrations be reset when a new image sequence is loaded? i.e. the user loads an image sequence, any existing registrations are removed/marked as invalid, and the GUI switches to No Registration?

How should this work for the Control Points option? Looking at the code you can set and save the control points regardless of the mode selected, but it will only run the transformation if the control points option is selected?

[ubdbra001]

If this becomes a headache it probably isn't worth it - we could probably lose too much tim to this for relatively little gain. I think really the most important thing here is just making sure there's no bug associated with image registration so that we're confident the data are being processed and displayed correctly (i.e. the priority is just fixing this issue, the rest is just added value that maybe isn't worth the time).

I agree, so it may be worth having a separate issue for enhancing the image registration options, that can be addressed later.

[twVolc]

This isn't quite the same issue but still relates to image registration being buggy. When I load a new image directory (straight after start-up with the default config file) then click to "No registration" the following error is thrown:

Exception in Tkinter callback
Traceback (most recent call last):
  File "C:\Users\gg1tcw\.conda\envs\pycam\lib\tkinter\__init__.py", line 1892, in __call__
    return self.func(*args)
  File "C:\Users\gg1tcw\Documents\PythonProjects\PyCamPermanent\pycam\gui\figures_cam.py", line 458, in <lambda>
    command=lambda: self.img_reg_select(self.reg_meth))
  File "C:\Users\gg1tcw\Documents\PythonProjects\PyCamPermanent\pycam\gui\figures_cam.py", line 550, in img_reg_select
    self.pyplis_worker.process_pair(img_path_A=None, img_path_B=None, plot=True, plot_bg=None, overwrite=True)
  File "C:\Users\gg1tcw\Documents\PythonProjects\PyCamPermanent\pycam\so2_camera_processor.py", line 3606, in process_pair
    img = img_A - self.test_img
UnboundLocalError: local variable 'img_A' referenced before assignment

The image does revert to no registration, so this isn't a breaking error, but it's not clear to me how, if at all, this affects future interactions. I think investigating and fixing this would be beneficial.

If I then click to control point registration (when we don't have one yet) I think this reverts back to No registration as the same error is thrown again.

[twVolc]

Following this, I went into the save menu to save a registration file. I then tried loading a new config file and got this error.

Traceback (most recent call last):
  File "C:\Users\gg1tcw\.conda\envs\pycam\lib\tkinter\__init__.py", line 1892, in __call__
    return self.func(*args)
  File "C:\Users\gg1tcw\Documents\PythonProjects\PyCamPermanent\pycam\gui\menu.py", line 772, in load_config_file
    self.reload_config()
  File "C:\Users\gg1tcw\Documents\PythonProjects\PyCamPermanent\pycam\gui\menu.py", line 780, in reload_config
    self.main_gui.menu.save_frame.load_defaults()
  File "C:\Users\gg1tcw\Documents\PythonProjects\PyCamPermanent\pycam\gui\misc.py", line 305, in load_defaults
    self.gather_vars()
  File "C:\Users\gg1tcw\Documents\PythonProjects\PyCamPermanent\pycam\gui\menu.py", line 905, in gather_vars
    tk.messagebox.showinfo('Save settings updated',
  File "C:\Users\gg1tcw\.conda\envs\pycam\lib\tkinter\messagebox.py", line 84, in showinfo
    return _show(title, message, INFO, OK, **options)
  File "C:\Users\gg1tcw\.conda\envs\pycam\lib\tkinter\messagebox.py", line 72, in _show
    res = Message(**options).show()
  File "C:\Users\gg1tcw\.conda\envs\pycam\lib\tkinter\commondialog.py", line 40, in show
    w = Frame(self.master)
  File "C:\Users\gg1tcw\.conda\envs\pycam\lib\tkinter\__init__.py", line 3124, in __init__
    Widget.__init__(self, master, 'frame', cnf, {}, extra)
  File "C:\Users\gg1tcw\.conda\envs\pycam\lib\tkinter\__init__.py", line 2572, in __init__
    self.tk.call(
_tkinter.TclError: bad window path name ".!toplevel"

I'm not sure if this is a result of the registration bug, or a result of me going into the save options window, or entirely unrelated to either of these and just a freak bug.

[twVolc]

Loading a control point registration also doesn't seem to do anything (unless the file I'm specifically using doesn't work - I can't attach it here). The registration stays as the current registration.

[ubdbra001]

This isn't quite the same issue but still relates to image registration being buggy. When I load a new image directory (straight after start-up with the default config file) then click to "No registration" the following error is thrown:

Exception in Tkinter callback
Traceback (most recent call last):
  File "C:\Users\gg1tcw\.conda\envs\pycam\lib\tkinter\__init__.py", line 1892, in __call__
    return self.func(*args)
  File "C:\Users\gg1tcw\Documents\PythonProjects\PyCamPermanent\pycam\gui\figures_cam.py", line 458, in <lambda>
    command=lambda: self.img_reg_select(self.reg_meth))
  File "C:\Users\gg1tcw\Documents\PythonProjects\PyCamPermanent\pycam\gui\figures_cam.py", line 550, in img_reg_select
    self.pyplis_worker.process_pair(img_path_A=None, img_path_B=None, plot=True, plot_bg=None, overwrite=True)
  File "C:\Users\gg1tcw\Documents\PythonProjects\PyCamPermanent\pycam\so2_camera_processor.py", line 3606, in process_pair
    img = img_A - self.test_img
UnboundLocalError: local variable 'img_A' referenced before assignment

The image does revert to no registration, so this isn't a breaking error, but it's not clear to me how, if at all, this affects future interactions. I think investigating and fixing this would be beneficial.

If I then click to control point registration (when we don't have one yet) I think this reverts back to No registration as the same error is thrown again.

Is this in the current dev branch? Or the draft PR branch #245 ? If it's the dev branch then I'm aware of it, this was the issue introduced by the changes in #195 I mentioned in our chat the other day. The changes I've made in the draft PR branch should fix this. If it's in the draft PR branch then I'll have a but more of a poke around.

[twVolc]

Ah ok, this was in dev sorry, so all good on that front!

[ubdbra001]

I'll still have a look at the other two things you mention next time I'm working on this issue, just to make sure I haven't missed anything

[ubdbra001]

Notes from our conversation about this earlier:

• No registration: No change, radio button always available
• Control points: Radio button disabled without CP registration, add run button to generate registration (to replace Save points buttons). Radio button enabled when registration generated/loaded.
• OpenCV: Pretty much identical to Control points (minus the bit about save control points button)

Additional info:

• No change to existing registrations when new sequence loaded
• Upon loading config:
  • If registration file (either for CP or OpenCV) is specified then load and use that
  • Otherwise just keep current registration

I think that's everything. Let me know if not.