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.

Sign up for GitHub

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

Jump to bottom

2nd round of modifications to improve/extend the Kapton correction tool. #1968

Merged

phyy-nx merged 7 commits into dials:main from voklejas2:fix_max_edge_pt2

Jan 21, 2022

Contributor

voklejas2 commented Jan 12, 2022

This round includes modifications to enable Kapton absorption correction
for higher angles of rotation of the Kapton tape.

Note: that in order to see the effects of these modifications, you must also change to cctbx_project branch 'katon_plugin_pt2'.


          2nd round of modifications to improve/extend the Kapton correction tool.

b5472e2

This round includes modifications to enable Kapton absorption correction
for higher angles of rotation of the Kapton tape.

voklejas2 marked this pull request as draft

January 12, 2022 16:58

Contributor Author

voklejas2 commented Jan 12, 2022 •

edited

Further explanation of what the code changes to 'kapton_2019_correction.py' do:

The logic in this module generates the Kapton absorption correction. This data is then used by the image viewer to render various aspects of this correction (in this case lines corresponding to absorption edge and max). To see the effects of the changes to 'kapton_2019_correction.py' you must use the plugin for the Kapton absorption correction. The module for this lives in the CCTBX repo.
Examples of behavior for various values of Kapton tape rotation angles:

Angle = 40.2 degrees with single panel (desired behavior)

Angle = 60.2 degrees with single panel (OK behavior, but the lines are drawn well past the boundary of the detector image)

Angle = -60.2 degrees with single panel (OK behavior, but the lines are drawn well past the boundary of the detector image)

Member

ndevenish commented Jan 12, 2022

Ah, so this is in the ImageViewer? the last newsfragment just said "Ability to Plot" and so I wasn't sure.


          add newsfragment

e73bddf

Contributor Author

voklejas2 commented Jan 12, 2022 •

edited

Ah, so this is in the ImageViewer? the last newsfragment just said "Ability to Plot" and so I wasn't sure.

Yeah, thanks for adding this comment because it might help clear up some confusion that others might have. Yes, ultimately all these modifications are to improve the Kapton plugin used by the image viewer. I may have inadvertently confused this issue because two different code bases are involved to make these improvements: the logic to render the effects of the Kapton absorption correction lives in CCTBX ('kapton_2019_correction_frame_frame_plugin.py') while the logic that generates the data used by the aforementioned plugin lives in dials (i.e., the file modified in this PR ' kapton_2019_correction.py').

Member

phyy-nx commented Jan 12, 2022

Yah, here's how you can look at it:

cd `libtbx.find_in_repositories dials`/util/image_viewer/slip_viewer
ln -s `libtbx.find_in_repositories cctbx_project`/xfel/util/kapton_2019_correction_frame_frame_plugin.py .
dials.data get lcls_rayonix_kapton
dials.image_viewer `libtbx.show_build_path`/dials_data/lcls_rayonix_kapton/hit-20181213155134902.cbf

Then click Actions->Show 2019 kapton tool. @voklejas2 is working on the 'show kapton absorption edge and max' button.

voklejas2 added 4 commits

January 12, 2022 09:44


          2nd round of modifications to improve/extend the Kapton correction tool.

230ee8d

This round includes modifications to enable Kapton absorption correction
for higher angles of rotation of the Kapton tape.


          add newsfragment

e5c336f


          Merge branch 'fix_max_edge_pt2' of github.com:voklejas2/dials into fi…

c5b1f41

…x_max_edge_pt2


          finalize changes from flake8

412b400

voklejas2 commented

View reviewed changes

algorithms/integration/kapton_2019_correction.py

@@ @@ -323,21 +323,9 @@ def _check_int_edge_pts(int_edge_pts): @@
                       # Sort out the edge points and the int_edge_points which are on the same side
                       kapton_edge_1 = (col(int_edge_pts[0]) - col(int_edge_pts[1])).normalize()
                       kapton_edge_2 = (col(int_edge_pts[2]) - col(int_edge_pts[3])).normalize()
-                      min_loss_func = -999.9

Contributor Author

voklejas2 Jan 12, 2022

I think the original intent of this now deleted logic was to reorder the indices of the detector edge points from counterclockwise to clockwise (starting from origin) based on how well the edges associated with kapton absorption max and min align with the sides of the detector. For example, if the kapton absortion edges are more parallel to the top/bottom sides of the detector, then the detector edge point are re-ordered to reflect that.

This logic was deleted because this re-ordering causes a divide by zero error on line 329

Contributor Author

voklejas2 Jan 12, 2022

I didn't attempt to re-implement a working version of this logic because I'm not certain if what the ultimate outcome the original logic sought to obtain or of its validity.

voklejas2 commented

View reviewed changes

algorithms/integration/kapton_2019_correction.py

@@ @@ -350,24 +338,14 @@ def _check_int_edge_pts(int_edge_pts): @@
                       # Now make sure the edges and the kapton lines are on the right side (i.e not swapped).
                       # Let's look at edge_idx[0:2] i,e the first edge of detector parallel to the kapton
                       pt1 = edge_pts[edge_idx[0]]
-                      pt2 = edge_pts[edge_idx[1]]

Contributor Author

voklejas2 Jan 12, 2022

unused variable

voklejas2 commented

View reviewed changes

algorithms/integration/kapton_2019_correction.py

                       # Now find the distance between each of these points and the kapton lines.
                       d1_kapton_1 = self.distance_of_point_from_line(
                           pt1, int_edge_pts[0], int_edge_pts[1]
                       )
                       d1_kapton_2 = self.distance_of_point_from_line(
                           pt1, int_edge_pts[2], int_edge_pts[3]
                       )
-                      d2_kapton_1 = self.distance_of_point_from_line(

Contributor Author

voklejas2 Jan 12, 2022 •

edited

Lines 361-366: unused variables

voklejas2 commented

View reviewed changes

algorithms/integration/kapton_2019_correction.py

		if d1_kapton_1 < d1_kapton_2: # closer to max than edge
		assert (

Contributor Author

voklejas2 Jan 12, 2022

This assertion breaks the ability to calculate absorption edge/max for higher angles of Kapton tape.

It seems to me that this now deleted logic is enforcing a relative orientation of the kapton tape absortion max and edge with detector edges that isn't necessarily always true. That said, I'm not sure what is lost by deleting this logic. I did find, however, that the deletion didn't result in any weird behavior at lower angles (negative or positive).


          Re-wrote and Re-factored some logic in abs_bounding_lines_in_mm

5c81933

voklejas2 commented

View reviewed changes

algorithms/integration/kapton_2019_correction.py

@@ @@ -217,6 +218,20 @@ def distance_of_point_from_line(self, r0, r1, r2): @@
                       denom = math.sqrt((y2 - y1) * (y2 - y1) + (x2 - x1) * (x2 - x1))
                       return abs(num) / denom
+                  def get_edge_distances(self, edges):

Contributor Author

voklejas2 Jan 18, 2022

re-factored redundant logic in abs_bounding_lines_in_mm into class method

voklejas2 commented

View reviewed changes

algorithms/integration/kapton_2019_correction.py

@@ @@ -242,6 +257,85 @@ def _check_int_edge_pts(int_edge_pts): @@
                                   new_int_edge_pts[3] = int_edge_pts[1]
                               return new_int_edge_pts
+                      def _orient_det_edge_pts(det_edge_pts):

Contributor Author

voklejas2 Jan 18, 2022

The original logic makes an unstated assumption that the detector edge points used to help calculate the location of the absorption max and edge (aka min) will be perpendicular to the y-axis. The original logic ran into edge cases where this was no longer true. This logic attempts to skirt this issue by enforcing the detector edge points to listed in a certain order based on location relative to the origin of the detector.

voklejas2 commented

View reviewed changes

algorithms/integration/kapton_2019_correction.py

+                              det_edge_pts[edge_3],
+                          ]
+                      def _make_kapton_distance_vec(

Contributor Author

voklejas2 Jan 18, 2022

Utility function used to help define the relative location of a given kapton intersection edge point relative to the detector center. This is useful when determining how to order the kapton intersection edge points when returning out of the abs_bounding_lines_in_mm function

voklejas2 commented

View reviewed changes

algorithms/integration/kapton_2019_correction.py

@@ @@ -250,18 +344,9 @@ def _check_int_edge_pts(int_edge_pts): @@
                           for point in [(0, 0), (0, s_size), (f_size, 0), (f_size, s_size)]:
                               x, y = panel.get_pixel_lab_coord(point)[0:2]
                               edges.append((x, y, detz))
-                      # Use the idea that the corners of the detector are end points of the diagonal and will be the

Contributor Author

voklejas2 Jan 18, 2022

replaced this original logic with call to class method

voklejas2 marked this pull request as ready for review

January 19, 2022 01:29

phyy-nx approved these changes

View reviewed changes

phyy-nx merged commit 0dab224 into dials:main

ndevenish mentioned this pull request

Update pre-commit environments #1986

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment