Create a simple use example for a camera #226

erinpougheon · 2024-07-15T13:27:56Z

Fixes #189.

Also tried to update the link for the Allied Vision cameras in the environment.yml file to link it to Vimba X VmbPy library.

erinpougheon · 2024-07-16T09:48:28Z

After creating a new environment as described in the README of catkit (but with a new name):
The GUI is working in simulation. There is only a camera viewer.

I tested the code on hardware too, on the THD2 testbed, with the visitor_camera (called science camera in the testbed_example of catkit). It worked as expected after the installation of vmbpy for the allied vision camera.
Here are two screenshots of what it looks like, when the flat is on and without the flat, with the laser on.

erinpougheon · 2024-07-16T14:45:51Z

For the Vimba update in the environment.yml, I let the previous link, because when I try to update it, I have a GitHub error, saying that it can be install from git (which in logical regarding the documentation of vmbpy of Vimba X). My suggestion might be to delete completly this line, but I'll wait for you opinion.

ivalaginja

Looking good! Just some small stuff to address.

The big question we need to answer here is whether we can move some of the GUI stuff into the actual catkit2 package. @erinpougheon how about you address the comments I left, and then we try to estimate together whether this is doable?

ivalaginja · 2024-07-16T13:17:27Z

catkit2/services/simple_simulator/simple_simulator.py

@@ -20,7 +20,7 @@ def open(self):

        self.images = self.make_data_stream('images', 'float64', self.model.focal_grid.shape, 20)

-        self.update_atmosphere()
+        self.update_atmosphere


Was this change intentional? self.atmosphere is a callable, so it usually should keep the parentheses in order to be called properly.

Yes it is. Before deleting the parenthesis, I had an error TypeError: update_atmosphere() takes 1 positional argument but 2 were given. It is the only solution I found to solve this problem.

I find that weird. Let me have a look at this and I'll come back to you, because currently I can't find an explanation for this behavior.

@ehpor any clue what might be happening here? @erinpougheon could you post the error message you're getting for this here please?

Hmmm I think you're avoiding the error, but I don't think the atmosphere works either. My intuition is that it just doesn't do anything.

If you look at the simulator.py code here, https://github.com/spacetelescope/catkit2/blob/a7c17df19636676151d965e67d0085367ba14769/catkit2/simulator/simulator.py#L131C13-L131C41, it calls the callback function with time as an argument. So it will call self.update_atmosphere(t), that has 2 arguments (self, and t). You would need to define def update_atmosphere(self, t) or something like that.

Also, looking at the code, I feel the simple_simulator on develop is bugged.

catkit2/services/simple_simulator/simple_simulator.py

testbed_example/cli_interface.py

testbed_example/config/services.yml

testbed_example/user_interface/bench_display.py

ivalaginja · 2024-07-16T14:59:24Z

testbed_example/user_interface/camera_viewer.py

+        vbox_sidebar.addWidget(self.tree)
+
+        # Set to values of camera
+        self.p.param('Exposure', 'ExposureTime').setValue(self.camera.exposure_time / 1e6)


This is something I want to address, although maybe not in this PR. This is a hard-coded conversion factor for the units of the camera exposure time, which is not fantastic in the example. Could you open an issue that says: "Remove hard-coded exposure time scaling in simple camera viewer" and link it here?

Here is the corresponding new issue : #228

testbed_example/user_interface/common.py

testbed_example/user_interface/logging_display.py

ivalaginja · 2024-07-16T15:04:22Z

For the Vimba update in the environment.yml, I let the previous link, because when I try to update it, I have a GitHub error, saying that it can be install from git (which in logical regarding the documentation of vmbpy of Vimba X). My suggestion might be to delete completly this line, but I'll wait for you opinion.

@erinpougheon ok. Can you open an issue for this ("Update Vimba Python package installation to VimbaX" or something like that) so that we don't forget about it?

erinpougheon · 2024-07-16T15:22:11Z

For the Vimba update in the environment.yml, I let the previous link, because when I try to update it, I have a GitHub error, saying that it can be install from git (which in logical regarding the documentation of vmbpy of Vimba X). My suggestion might be to delete completly this line, but I'll wait for you opinion.

@erinpougheon ok. Can you open an issue for this ("Update Vimba Python package installation to VimbaX" or something like that) so that we don't forget about it?

Done, here is the new issue : #229

erinpougheon · 2024-07-16T15:26:25Z

I changed everything you requested !

ivalaginja

One mini thing to fix, please, and the other comment is more a note to myself - consider this PR done from your side, and I will try to check why I find that one callable call weird.

testbed_example/user_interface/bench_display.py

ivalaginja · 2024-07-16T15:31:01Z

catkit2/services/simple_simulator/simple_simulator.py

@@ -20,7 +20,7 @@ def open(self):

        self.images = self.make_data_stream('images', 'float64', self.model.focal_grid.shape, 20)

-        self.update_atmosphere()
+        self.update_atmosphere


I find that weird. Let me have a look at this and I'll come back to you, because currently I can't find an explanation for this behavior.

ivalaginja · 2024-07-16T20:06:34Z

While trying to figure out what is happening with the update_atmosphere() method, I saw that the input to adding a callback function is not only the wrong way around where you fixed it, but also in the DM method:

catkit2/catkit2/services/simple_simulator/simple_simulator.py

Line 30 in 5c2f7a9

self.add_callback(at_time, callback)

Could you fix that one as well please? It is not used in the example you set up so we didn't notice, but since we're already on it, let's fix that one too.

erinpougheon · 2024-07-19T11:49:31Z

@ivalaginja changed the simulator and optical model used in the example, so we don't have the same problem with the update_atmosphere(). (I think we can maybe open a new issue to just adress this simulator problem, I don't know).

Now, the example is working in simulation (evn though there is no light on it, because light is not implmented in the new example of simulation). Here is the result on the simulated server:

Hardware test on the THD2 testbed: Everything works as expected. I was able to have the viewer (we can see things because I turned on a laser). I could change the ROI, the gain, the exposure time etc.

I think this PR is ready for final review and can be merged. I added the parenthesis back in the update_atmosphere function, in case we want it to be a separate issue.

ivalaginja · 2024-07-19T13:42:40Z

testbed_example/example_optical_model.py

+    @property
+    def focal_grid(self):
+        return hcipy.make_focal_grid(12, 32)
+


Do you think wherever you needed the focal_grid you could just use the detector_grid below? I would like the grid parameters to be read from the configuration, and I was trying to call things in the detector plane (where the focal grid is) with "detector" to keep things more or less consistent.

ivalaginja · 2024-07-19T13:44:51Z

testbed_example/services/example_simulator/example_simulator.py

@@ -12,7 +12,7 @@ def __init__(self):
        super().__init__('example_simulator')

    def open(self):
-        self.model = ExampleOpticalModel()
+        self.model = ExampleOpticalModel(self.testbed.config)


Nice catch on this - I also think it needs to be self.testbed.config['simulator']? Otherwise I am not sure what pool of configurations it tries to read, between all the different configuration files the example has. It might be automatically choosing the right one, but it's better to be explicit.

I made it go on the simulator file in the optical model, but I can defenitly change that you're right.

erinpougheon · 2024-07-19T14:01:15Z

Here is the viewer (in simulation, because we changed nothing that has to do with the hardware) after the changes, everything works.

ivalaginja

Good by me. Please wait with merging until we hear back from @raphaelpclt.

raphaelpclt

The code looks good overall. A few things can be removed (like experiment paths in the config - catkit2 doesn't implement experiment structures). Still pending discussion on what to let public or not.

raphaelpclt · 2024-07-23T18:55:25Z

testbed_example/cli_interface.py

+Options:
+  -p, --port PORT_ID      The port number on which the testbed server operates.
+                          Defaults to the port specified in the config file.
+  --simulated             Whether the testbed server should be run in simulated mode or not.


I am not sure - shouldn't --simulated be the default option or hardcoded? This example will never be running not simulated I guess.

erinpougheon added the enhancement New feature or request label Jul 15, 2024

erinpougheon self-assigned this Jul 15, 2024

ivalaginja mentioned this pull request Jul 16, 2024

Decide if the functions in the __init__.py of the testbed_example config folder should be moved into catkit2 package #227

Closed

erinpougheon requested a review from ivalaginja July 16, 2024 13:11

erinpougheon changed the title ~~Create an simple use example for a camera~~ Create a simple use example for a camera Jul 16, 2024

erinpougheon force-pushed the feature/simple_example_camera branch from f0db62e to 970bfc9 Compare July 16, 2024 14:46

ivalaginja reviewed Jul 16, 2024

View reviewed changes

This was referenced Jul 16, 2024

Remove hard-coded exposure time scaling in simple camera viewer #228

Closed

Update Vimba Python package installation to VimbaX #229

Open

erinpougheon requested a review from ivalaginja July 16, 2024 15:26

ivalaginja requested changes Jul 16, 2024

View reviewed changes

erinpougheon mentioned this pull request Jul 17, 2024

Create a Fourier Transform Viewer #230

Closed

erinpougheon force-pushed the feature/simple_example_camera branch from a7c17df to aaea162 Compare July 18, 2024 12:18

ivalaginja mentioned this pull request Jul 19, 2024

Add example optical model and example simulator #233

Merged

erinpougheon requested a review from ivalaginja July 19, 2024 12:49

ivalaginja reviewed Jul 19, 2024

View reviewed changes

ivalaginja requested a review from raphaelpclt July 19, 2024 13:56

erinpougheon requested a review from ivalaginja July 19, 2024 14:05

ivalaginja previously approved these changes Jul 19, 2024

View reviewed changes

raphaelpclt reviewed Jul 24, 2024

View reviewed changes

ivalaginja dismissed their stale review via 8f235c3 August 2, 2024 13:27

ivalaginja force-pushed the feature/simple_example_camera branch from 926695f to 8f235c3 Compare August 2, 2024 13:27

ivalaginja closed this Aug 2, 2024

ivalaginja force-pushed the feature/simple_example_camera branch from 8f235c3 to fcce243 Compare August 2, 2024 13:29

spacetelescope deleted a comment from raphaelpclt Aug 2, 2024

ivalaginja deleted the feature/simple_example_camera branch August 2, 2024 13:31

ivalaginja mentioned this pull request Oct 24, 2024

Remove VimbaPython dependency from environment.yml #257

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create a simple use example for a camera #226

Create a simple use example for a camera #226

erinpougheon commented Jul 15, 2024 •

edited

Loading

erinpougheon commented Jul 16, 2024 •

edited

Loading

erinpougheon commented Jul 16, 2024

ivalaginja left a comment

ivalaginja Jul 16, 2024

erinpougheon Jul 16, 2024

ivalaginja Jul 16, 2024

ivalaginja Jul 17, 2024

erinpougheon Jul 18, 2024

raphaelpclt Jul 18, 2024 •

edited

Loading

ivalaginja Jul 16, 2024

erinpougheon Jul 16, 2024

ivalaginja commented Jul 16, 2024

erinpougheon commented Jul 16, 2024

erinpougheon commented Jul 16, 2024

ivalaginja left a comment

ivalaginja Jul 16, 2024

ivalaginja commented Jul 16, 2024

erinpougheon commented Jul 19, 2024 •

edited

Loading

ivalaginja Jul 19, 2024

ivalaginja Jul 19, 2024 •

edited

Loading

erinpougheon Jul 19, 2024

erinpougheon commented Jul 19, 2024 •

edited

Loading

ivalaginja left a comment

raphaelpclt left a comment

raphaelpclt Jul 23, 2024

Create a simple use example for a camera #226

Create a simple use example for a camera #226

Conversation

erinpougheon commented Jul 15, 2024 • edited Loading

erinpougheon commented Jul 16, 2024 • edited Loading

erinpougheon commented Jul 16, 2024

ivalaginja 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

raphaelpclt Jul 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivalaginja commented Jul 16, 2024

erinpougheon commented Jul 16, 2024

erinpougheon commented Jul 16, 2024

ivalaginja left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivalaginja commented Jul 16, 2024

erinpougheon commented Jul 19, 2024 • edited Loading

Choose a reason for hiding this comment

ivalaginja Jul 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erinpougheon commented Jul 19, 2024 • edited Loading

ivalaginja left a comment

Choose a reason for hiding this comment

raphaelpclt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erinpougheon commented Jul 15, 2024 •

edited

Loading

erinpougheon commented Jul 16, 2024 •

edited

Loading

raphaelpclt Jul 18, 2024 •

edited

Loading

erinpougheon commented Jul 19, 2024 •

edited

Loading

ivalaginja Jul 19, 2024 •

edited

Loading

erinpougheon commented Jul 19, 2024 •

edited

Loading