poi manager: z-position from the scanner is not saved #647

rmgonzal · 2021-03-24T12:37:52Z

Z-position from the scanner is not saved when you add a point of interest (POI)

Description

This bug is affected because POI on the confocal image only saves the x, y position into the roi_origin, and since, by default, the values are set to zero. Since the z position is not saved into the POI file (which should be a fixed value) and take it from ROI origin.
So simply adding new_pos[2] = self.poimanagerlogic().scanner_position[2] in line 629 "gui/poimanger/poimangui.py" will fix this bug.

Motivation and Context

The main motivation to fix this problem is to have the right focal plane for your POI file. This bug is caused when you select a point by clicking into the poi_manager, by default, the values before clicking are set to zero (np.zeros(3)), so when you select more than one POI the Z position is not stored.

So simply adding new_pos[2] = self.poimanagerlogic().scanner_position[2] line 629 "gui/poimanger/poimangui.py" will fix this bug.

#512 (comment)

How Has This Been Tested?

This has been tested by me in two different confocal setups

You can test this by adding more than one POI and checking the file in the console

In[1]: poimanagerlogic.poi_positions

and the last value should be different from zero (e.g. 5 points same Z pos value).

Out[1]: 
{'poi_20210322102945044728': array([3.67941896e-05, 5.56220471e-05, 5.00000000e-05]),
 'poi_20210322102946548945': array([3.15235204e-05, 3.33681106e-05, 5.00000000e-05]),
 'poi_20210322102947693560': array([7.01750944e-05, 5.21082677e-05, 5.00000000e-05]),
 'poi_20210324133553994647': array([2.09481774e-05, 7.84616136e-05, 5.00000000e-05]),
 'poi_20210324133554617894': array([1.27493587e-05, 5.09370079e-05, 5.00000000e-05])}

Screenshots (only if appropriate, delete if not):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

My code follows the code style of this project.
I have documented my changes in the changelog (documentation/changelog.md)
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added/updated for the module the config example in the docstring of the class accordingly.
I have checked that the change does not contain obvious errors (syntax, indentation, mutable default values).
I have tested my changes using 'Load all modules' on the default dummy configuration with my changes included.
All changed Jupyter notebooks have been stripped of their output cells.

#512 (comment) This bug is affected because POI on the confocal image only saves the x, y position into the roi_origin, and since, by default, the values are set to zero. Since the z position is not saved into the POI file (which should be a fixed value) and take it from ROI origin. So simply adding new_pos[2] = self.poimanagerlogic().scanner_position[2] line 629 "gui/poimanger/poimangui.py" will fix this bug.

NicolasStaudenmaier

Looks good to me. I will test it soon.
The question remains if this is a nice solution or if it is better to save the z-position of the scan as well and work (as I assume it is supposed to) with the sample shift (roi_origin) in z. In any case this is a working solution and perfectly fine for me.

NicolasStaudenmaier · 2021-03-24T12:50:33Z

gui/poimanager/poimangui.py

@@ -626,6 +626,8 @@ def create_poi_from_click(self, button, pos):
        new_pos = self.poimanagerlogic().roi_origin


This line is not needed anymore. Either initialize an empty array here or define the new position in one line like
new_pos = np.array([pos.x(), pos.y(), self.poimanagerlogic().scanner_position[2]])

Neverhorst · 2021-03-24T13:25:02Z

@NicolasStaudenmaier You have recognized the problem here. The ROI origin is the anchor for all POIs registered. So in the beginning the "offset" introduced by sample shift etc. is zero, so the absolute position of an POI is simply the POI initial position. If you now moved the ROI (e.g. by tracking a POI or updating the ROI origin manually), the POI position will become the sum of the POI initial position and the ROI origin (now different from [0, 0, 0]).
If you add a new POI with the current absolute scanner position, the origin is substracted from this position to normalize all POIs to zero sample shift/origin.
So, I'm not sure if this solution works if you add POIs by mouse click in the ROI if you introduce sample shifts in between (create ROI -> shift -> new POI -> shift -> new POI -> shift -> ...). Could be that the origin needs to be subtracted or added for X and Y if adding a POI via mouse click inside the scan image... needs to be checked.

In any case please do not introduce a new absolute coordinate (for Z) to be handled by the POI manager. This is conceptually not needed.

NicolasStaudenmaier · 2021-03-24T15:04:44Z

It behaves well when adding POIs by mouse clicks, introducing a sample shift and repeating the procedure. When creating an POI the anchor position is calculated by subtracting the shift from the absolute position.

So to fix the issue without introducing an absolute z-coordinate in the POI manager, I believe that the add_poi function in the poimanagerlogic needs to be adapted to accept 2D position inputs (x and y) and taking the z-position from the current scanner position. Then x and y are emitted with the signal upon adding a POI by click and the z-position will be assigned from the poimanagerlogic.

rmgonzal · 2021-03-29T13:10:36Z

The idea was to solve this issue. The roi_origin is related to another issue (shift of each track ->update POI). Still, in this case, it gets a bit confused because the POI's shift will be in 3D, and when you select the POI's, you click them in a 2D confocal image, so in principle, the Z position is fixed.

So I updated as @NicolasStaudenmaier @Neverhorst suggest

625 # Z position from scanner, X and Y positions from click event
626 new_pos =  np.array([pos.x(), pos.y(), self.poimanagerlogic().scanner_position[2]])

rmgonzal changed the title ~~Update poimangui.py~~ poi manager: z-position from the scanner is not saved Mar 24, 2021

NicolasStaudenmaier linked an issue Mar 24, 2021 that may be closed by this pull request

Bug to be fixed: When creating a poi by mouse click in poi manager gui, would creat a poi with its z-axis value is zero. #512

Open

3 tasks

NicolasStaudenmaier reviewed Mar 24, 2021

View reviewed changes

Update poimangui.py

c5f3f94

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

poi manager: z-position from the scanner is not saved #647

poi manager: z-position from the scanner is not saved #647

rmgonzal commented Mar 24, 2021 •

edited

NicolasStaudenmaier left a comment

NicolasStaudenmaier Mar 24, 2021

Neverhorst commented Mar 24, 2021

NicolasStaudenmaier commented Mar 24, 2021

rmgonzal commented Mar 29, 2021

		@@ -626,6 +626,8 @@ def create_poi_from_click(self, button, pos):
		new_pos = self.poimanagerlogic().roi_origin

poi manager: z-position from the scanner is not saved #647

Are you sure you want to change the base?

poi manager: z-position from the scanner is not saved #647

Conversation

rmgonzal commented Mar 24, 2021 • edited

Description

Motivation and Context

How Has This Been Tested?

Screenshots (only if appropriate, delete if not):

Types of changes

Checklist:

NicolasStaudenmaier left a comment

Choose a reason for hiding this comment

NicolasStaudenmaier Mar 24, 2021

Choose a reason for hiding this comment

Neverhorst commented Mar 24, 2021

NicolasStaudenmaier commented Mar 24, 2021

rmgonzal commented Mar 29, 2021

rmgonzal commented Mar 24, 2021 •

edited