Standalone SpatialOrientation #44

braden6521 · 2024-03-21T18:34:22Z

Split SpatialOrientation from DisplayShape

Removed the spatial orientation (relative camera-screen positions) content from the DisplayShape object. These are not dependent on each other and do not need to be stored together.
This is how SofastFixed requires its information. SofastFixed doesn't always need a DisplayShape object, but always requires a SpatialOrientation. (Thus, this was not a problem before SofastFixed existed.)
This also streamlines the calibration processes. Spatial orientation information does not need to be saved as an intermediate CSV file; the camera-pose and screen-shape calibrations are now stand-alone calibrations.
Deleted everything related to the function save_DisplayShape_file because this is no longer needed.
The unit test data had to be rearranged (there is no rvec/tvec stored in the DisplayShape file). The spatial orientation data is now stored in the Sofast dataset HDF files under the folder SpatialOrientation/...
Added more unit test coverage where appropriate.
Updated/removed Sofast files in /doc

bbean23

Please respond to my comments before I approve. In particular, my comments about scrubbing, test_Display.test_save_load_hdf_dist_3d(), and CalibrateDisplayShape.as_DisplayShape()

bbean23 · 2024-03-25T18:12:51Z

doc/source/example/camera_calibration/config.rst

@@ -0,0 +1,8 @@
+View Camera Distortion


I need to learn what these .rst files do at some point.

bbean23 · 2024-03-25T18:20:33Z

opencsp/app/sofast/lib/CalibrateDisplayShape.py

+        return {'xy_screen_fraction': pts_xy_screen_fraction, 'xyz_screen_coords': pts_xyz_screen}
+
+    def as_DisplayShape(self, name: str) -> DisplayShape:
+        """Saves data to DisplayShape hdf file using the distorted3d model, see


This doesn't actually save the data, does it? It just returns a DisplayShape instance?

Nope, that was a copy/paste error.

bbean23 · 2024-03-25T18:22:16Z

opencsp/app/sofast/lib/DisplayShape.py


 from opencsp.common.lib.geometry.Vxy import Vxy
 from opencsp.common.lib.geometry.Vxyz import Vxyz
-import opencsp.common.lib.tool.hdf5_tools as hdf5_tools
+import opencsp.common.lib.tool.hdf5_tools as ht


We should probably converge on how we import this library. I've been using "import ... as h5", but I'm also good with "ht". I think that "h5" is more descriptive, but "ht" is more consistent with the rest of the code. Thoughts?

Good idea on being consistent. Since you're already using h5, let's stick with that.

Updated to ... as h5 in all (two) files.

bbean23 · 2024-03-25T18:23:41Z

opencsp/app/sofast/lib/ProcessSofastFringe.py

@@ -11,7 +11,7 @@
 from opencsp.app.sofast.lib.DefinitionFacet import DefinitionFacet
 from opencsp.app.sofast.lib.DisplayShape import DisplayShape as Display
 import opencsp.app.sofast.lib.image_processing as ip
-from opencsp.app.sofast.lib.MeasurementSofastFringe import MeasurementSofastFringe as Measurement
+from opencsp.app.sofast.lib.MeasurementSofastFringe import MeasurementSofastFringe


I like this change 👍

bbean23 · 2024-03-25T18:30:41Z

opencsp/app/sofast/test/test_DisplayShape.py

+        disp.save_to_hdf(file)
+
+        # Load
+        DisplayShape.load_from_hdf(file)


I suggest expanding this test method to compare the saved and loaded versions of the DisplayShape object.

Good idea, updated!

bbean23 · 2024-03-25T18:37:37Z

opencsp/app/sofast/test/test_SpatialOrientation.py

+        # Load
+        ori = SpatialOrientation.load_from_hdf(file)
+        # Check optic is oriented
+        np.testing.assert_equal(ori.optic_oriented, True)


Exactly! Like this! :)

Note that if you're comparing two standard python objects, you can also use self.assertEqual(a, b), which in my opinion produces easier-to-read results in the case that the two instance's aren't equal. In reality, it's just a matter of opinion which one you like more.

Example of np.testing.assert_equal and unittest.TestCase.assertEqual

I like that method a lot better. Updated!

bbean23 · 2024-03-25T18:43:51Z

opencsp/test/data/measurements_sofast_fringe/calculations_facet/data.h5

Did this file get run through the scrubber? I'm assuming it's safe, but we should make sure it passes the scrubber's checks, anyways.

Same with the rest of the HDF5 files...

Was never able to get the scrubber working/do not know how to use it. Will work on this.

braden6521 · 2024-03-25T21:51:24Z

@bbean23, now the ubi8 CI doesn't work. It started failing on memory_monitor for some reason FYI.

Fixed merge conflict with ProcessSofastFringe

Modified io, docstrings, and tests of DisplayShape Resolved merge conflicts in DisplayShape and test_Displayshape

Resolved merge conflict in test_integration_undefined

Fixed merge conflicts after rebase.

updated example file.

…ientation object

braden6521 · 2024-03-29T05:19:46Z

@bbean23, I believe this is finally ready after a rebase on top of develop! Nothing substantial has changed since your last review.

bbean23

Approved!

braden6521 added the enhancement New feature or request label Mar 21, 2024

braden6521 changed the title ~~Spatial orientation standalone object~~ Standalone SpatialOrientation Mar 21, 2024

braden6521 force-pushed the spatial_orientation branch 2 times, most recently from 1672d64 to 76ed5ec Compare March 21, 2024 21:42

braden6521 requested a review from bbean23 March 21, 2024 22:15

braden6521 marked this pull request as ready for review March 21, 2024 22:15

braden6521 removed the request for review from bbean23 March 21, 2024 22:34

braden6521 marked this pull request as draft March 21, 2024 22:35

braden6521 force-pushed the spatial_orientation branch from f6cd54c to b27df4e Compare March 25, 2024 15:38

braden6521 marked this pull request as ready for review March 25, 2024 16:31

braden6521 requested a review from bbean23 March 25, 2024 16:31

bbean23 requested changes Mar 25, 2024

View reviewed changes

braden6521 force-pushed the spatial_orientation branch 3 times, most recently from 165fe76 to 7ccd13c Compare March 28, 2024 15:20

braden6521 added 14 commits March 28, 2024 21:59

Remove spatial orientation data from DisplayShape

ce8aa59

Fixed merge conflict with ProcessSofastFringe

Updated DisplayShape and SpatialOrientation io

ed615a8

B

870766e

Modified io, docstrings, and tests of DisplayShape Resolved merge conflicts in DisplayShape and test_Displayshape

Updating test_integration_undefined

058a260

Resolved merge conflict in test_integration_undefined

Updated calibrate screen shape example

d8a1638

Fixed merge conflicts after rebase.

Added saving io tests to DisplayShape

ac86782

Updated CalibrateDisplayShape to return DisplayShape object directly and

d882d75

updated example file.

Updated documentation in example-calibration-camera-pose

94348f4

Deleted example-calibration-save-DisplayShape-file

1412cec

Deleted save_DisplayShape file

9f76a59

Updated test_CalibrateDisplayShape unit test

b95fcdf

Added IO test to calibrate display shape unit test

7b217ce

Updated HDF5 test data files to contain SpatialOrientation.

48fed69

Fixed test spatial orientation

d4cee69

braden6521 added 20 commits March 28, 2024 22:05

Updated test integration single facet

c92d96e

Updated test integration multifacet and assiciated data file

efd2a6a

Updated test-spatial-processing and test-slopeSolver to use SpatialOr…

ba17214

…ientation object

Renamed sofast fringe example to spatial-orientation

e517170

Added HDF5 IO Abstract class inheritance to SpatialOrientation

088caed

Edited spatial orientation HDF IO.

645b5a7

Updated SO unit test

189abe9

Updated unit test data

5a446da

Added the creation of save directories if needed in unit tests

16efbfb

Fixed unit tests, made save dirs when necessary, sorted glob lists.

7856e29

Sorted all glob lists

7389f32

Formatted four files to be Black compliant.

ea3da16

Added/removed sofast doc files to pass doc checks.

b248c0a

Updated CalibrateDisplayShape documentation.

e9fe47c

Changed as ht to as h5 in imports.

3c308a5

Modified unit test functionalities.

c7a3cbd

Updates to be black compliant.

4fe084c

Added optic_oriented to spatial_orientation unit test data.

e724825

Changed hf to h5 in test_ProcessSofastFixed

cd8dbb3

Changed how spatial orientation is saved in calculation data classes.

f11738c

braden6521 force-pushed the spatial_orientation branch from 92840c7 to f11738c Compare March 29, 2024 04:41

braden6521 added 2 commits March 28, 2024 22:52

Removed unnecessary save_data_as_hdf method from CalibrateDisplayShape

358ed5a

Updated sofastFringe examples to use Spatial Orientation object.

de95ab2

bbean23 approved these changes Mar 29, 2024

View reviewed changes

braden6521 merged commit fee821d into sandialabs:develop Mar 29, 2024
4 checks passed

braden6521 deleted the spatial_orientation branch April 15, 2024 19:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Standalone SpatialOrientation #44

Standalone SpatialOrientation #44

braden6521 commented Mar 21, 2024 •

edited

Loading

bbean23 left a comment

bbean23 Mar 25, 2024

bbean23 Mar 25, 2024

braden6521 Mar 25, 2024

bbean23 Mar 25, 2024

braden6521 Mar 25, 2024

braden6521 Mar 25, 2024

bbean23 Mar 25, 2024

bbean23 Mar 25, 2024

braden6521 Mar 25, 2024

bbean23 Mar 25, 2024

braden6521 Mar 25, 2024

bbean23 Mar 25, 2024

bbean23 Mar 25, 2024

braden6521 Mar 25, 2024

braden6521 commented Mar 25, 2024

braden6521 commented Mar 29, 2024

bbean23 left a comment

Standalone SpatialOrientation #44

Standalone SpatialOrientation #44

Conversation

braden6521 commented Mar 21, 2024 • edited Loading

Split SpatialOrientation from DisplayShape

bbean23 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

braden6521 commented Mar 25, 2024

braden6521 commented Mar 29, 2024

bbean23 left a comment

Choose a reason for hiding this comment

braden6521 commented Mar 21, 2024 •

edited

Loading