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

Sofast fixed cal update #193

Merged
merged 19 commits into from
Dec 18, 2024

Conversation

braden6521
Copy link
Collaborator

@braden6521 braden6521 commented Dec 10, 2024

Purpose

Added more debug options to SofastFixed calibration routine to make diagnosing problems with calibration easier.

Summary of changes

  • Made a DebugBlobIndex class to hold debug options for finding blobs.
  • Added debug options to BlobIndex to better help debugging.
  • Fixed error where x/y shapes were swapped in CalibrateSofastFixedDots (does not affect calculation except in largely rectangular screen shapes).
  • Added mirror corner visualization to ensemble definition class
  • Made SofastConfiguration compatible with SofastFixed measurements
  • Added a CalculationBlobAssignment class to store outputs from finding blobs during a SofastFixed assignment.

Implementation notes

All updates were debugging-related changes. All unit tests were verified. In one case, a line-color of an output plot changed; this was updated.

Submission checklist

  • Target branch is develop, not main
  • Existing tests are updated or new tests were added
  • opencsp/test/test_DocStringsExist.py are verified to include this change or have been updated accordingly
  • .rst file(s) under doc/ are verified to include this change or have been updated accordingly

Additional information

(NA)
Please provide any additional information here.

bbean23
bbean23 previously approved these changes Dec 11, 2024
Copy link
Collaborator

@bbean23 bbean23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved! Feel free to take my moments or leave them as you see appropriate.

self.figures: list = []
self.bg_image: np.ndarray = None
self.name: str = ''

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest adding a destructor method that closes all the figures held onto by this instance. Maybe consider extending https://github.com/sandialabs/OpenCSP/blob/develop/opencsp/common/lib/render/lib/AbstractPlotHandler.py for this functionality.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@@ -140,7 +140,7 @@ def __init__(
self._dot_image_points_indices: Vxy
self._dot_image_points_indices_x: ndarray
self._dot_image_points_indices_y: ndarray
self._dot_points_xyz_mat = np.ndarray((x_max - x_min + 1, y_max - y_min + 1, 3)) * np.nan
self._dot_points_xyz_mat = np.ndarray((y_max - y_min + 1, x_max - x_min + 1, 3)) * np.nan
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my own edification, what does multiplying by np.nan accomplish?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those matrices get filled in with data as the algorithm finds dots. Since some values may actually be zero, we need some other value to define "no data." NaN seemed appropriate.

@@ -8,10 +8,15 @@ class ParamsOpticGeometry(hdf5_tools.HDF5_IO_Abstract):
"""Parameter dataclass for processing optic geometry"""

perimeter_refine_axial_search_dist: float = 50.0
"""The length of the search box (along the search direction) to use when finding optic perimeter. Default 50.0"""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for adding comments! Are these in units of pixels?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes and done!

@braden6521 braden6521 marked this pull request as ready for review December 13, 2024 21:00
…ed. Only became a problem when dot array was not square.
…s_optic_singlefacet_geometry to be compatible with necessary changes to process_optic_multifacet_geometry.
@braden6521 braden6521 force-pushed the sofast_fixed_cal_update branch from 65c14f0 to f26cde1 Compare December 13, 2024 21:02
@e10harvey e10harvey merged commit 6acd656 into sandialabs:develop Dec 18, 2024
4 checks passed
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 this pull request may close these issues.

3 participants