Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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 done
diamond2nv opened this issue May 6, 2019 · 1 comment · May be fixed by #647
Assignees

Comments

@diamond2nv
Copy link

diamond2nv commented May 6, 2019

What is affected by this bug?

  • Win10 64bit
  • Win10 . 1809
  • Qudi branch - scanimage_plotwidget_improvement (merged into master 2019-04-15)
  • gui/poimanger/poimangui.py

When does this occur?

  • When creating a poi by mouse click in poi manager gui, would creat a poi with its z-axis value is zero.

Where on the platform does it happen?

  • gui/poimanger/poimangui.py -- def create_poi_from_click() .
  • (Just find POI z=0. in saved ROI file)

How do we replicate the issue?

  1. qudi/example/default.cfg, open confocal scan gui
  2. move z-axis to 50.0 to begin scanning
  3. open POI Manager GUI, and get confocal scan ROI
  4. add some poi created by mouse clicked
  5. save ROI
  6. open saved ROI file, would find POI position z is 0.0, which should be 50.0

Expected behavior (i.e. solution)

  • Solution1: z-axis value of the poi created by mouse clicked, would be set as same as scanner z position, below code should be a quick fix:
    line 572 in "gui/poimanger/poimangui.py"
            #TODO: Z position from ROI origin is 0.
            new_pos = self.poimanagerlogic().roi_origin
            new_pos[2] = self.poimanagerlogic().scanner_position[2]
  • Solution2: Not so sure, but would restore clicked poi z-value from shifted ROI:
    line 572 in "gui/poimanger/poimangui.py"
            #TODO: restore z-axis value from shift origin postition ?
            new_pos = self.poimanagerlogic().roi_origin
            new_pos[2] = self.poimanagerlogic().scanner_position[2] -  new_pos[2]

Other Comments

  • Does "roi_origin" most likely mean as "default shift"? Since its default value is np.zeros(3)
@rmgonzal rmgonzal self-assigned this Mar 22, 2021
@rmgonzal
Copy link

This bug is affected because, as you mentioned, when you Add POI's or click a new POI on the confocal image, it only saves the x, y position into the roi_origin, and since, by default, the values are set to zero.
The z position is not saved into the POI file, which in this case, should be a fixed value (focal plane e.g. Z_pos = 50.00) and take it from ROI origin.

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

625        # Z position from ROI origin, X and Y positions from click event
626        new_pos = self.poimanagerlogic().roi_origin
627        new_pos[0] = pos.x()
628        new_pos[1] = pos.y()
629        new_pos[2] = self.poimanagerlogic().scanner_position[2]  #Add this line

To check if the value is stored in the POI file, check on the console typing.

In[1]: poimanagerlogic.poi_positions

and the last value should be different from zero.
(Example 3 point)

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])}

rmgonzal added a commit that referenced this issue Mar 24, 2021
#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.
@rmgonzal rmgonzal linked a pull request Mar 24, 2021 that will close this issue
11 tasks
@NicolasStaudenmaier NicolasStaudenmaier linked a pull request Mar 24, 2021 that will close this issue
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants