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

Moving from Calibration to Volume mode loses focus #142

Open
fedem-p opened this issue Jul 10, 2022 · 16 comments
Open

Moving from Calibration to Volume mode loses focus #142

fedem-p opened this issue Jul 10, 2022 · 16 comments
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@fedem-p
Copy link
Member

fedem-p commented Jul 10, 2022

I know this is probably related to #103 and #104, But I though it would have been good to have a single thread to discuss this specific problem.

Expected Behavior

After correctly calibrating the scanners, if mode is switched to planar or volume the preview should remain in focus.

Actual behavior

After correctly calibrating the scanners, if switching mode to planar the live view is in focus as expected.
However, the focus is lost in the volumetric mode.

Exploration of exact behavior of the scanners

I've spent quite some time reading the commands sent to piezo and lateral scanner to further understand what caused this problem.
focus_bug
In this image there are plotted the piezo values against the lateral scanning.
The legend is explained as follows:

  • 0.5 Hz acquisition frequency
  • 1 Hz acquisition frequency
  • 2 Hz acquisition frequency
  • 4 Hz acquisition frequency
  • test is the same as 4 Hz but with an hard-coded offset to try fixing the curve
  • planar is the line fitted taking a couple of points in planar mode

Ideally the part of the wave of the piezo vs lateral where we send the camera pulses should match in planar and volumetric mode.

Where is this in the code?

The main place of divergence from planar mode and volumetric mode is inside the scanloops module.
In particular, there are a couple of functions which seem to impact the reading and writing of the piezo.

So far I've mostly looked into the fill_arrays function:

   def fill_arrays(self):
        super().fill_arrays()
        self.board.z_piezo = self.z_waveform.values(self.shifted_time)
        i_sample = self.i_sample % len(self.recorded_signal.buffer)

        if self.recorded_signal.is_complete():
            wave_part = self.recorded_signal.read(i_sample, self.n_samples)
            max_wave, min_wave = (np.max(wave_part), np.min(wave_part))
            if (
                -2 < calc_sync(min_wave, self.parameters.z.lateral_sync) < 2
                and -2 < calc_sync(max_wave, self.parameters.z.lateral_sync) < 2
            ):
                self.board.z_lateral = calc_sync(
                    wave_part, self.parameters.z.lateral_sync
                )
            if (
                -2 < calc_sync(min_wave, self.parameters.z.frontal_sync) < 2
                and -2 < calc_sync(max_wave, self.parameters.z.frontal_sync) < 2
            ):
                self.board.z_frontal = calc_sync(
                    wave_part, self.parameters.z.frontal_sync
                )

        camera_pulses = 0
        if self.camera_on:
            self.logger.log_message("I")
            if self.camera_was_off:
                self.logger.log_message("Camera was off")
                # calculate how many samples are remaining until we are in a new period
                if i_sample == 0:
                    camera_pulses = self.camera_pulses.read(i_sample, self.n_samples)
                    self.camera_was_off = False
                    self.wait_signal.clear()
                else:
                    n_to_next_start = self.n_samples_period() - i_sample
                    if n_to_next_start < self.n_samples:
                        camera_pulses = self.camera_pulses.read(
                            i_sample, self.n_samples
                        ).copy()
                        camera_pulses[:n_to_next_start] = 0
                        self.camera_was_off = False
                        self.wait_signal.clear()
            else:
                camera_pulses = self.camera_pulses.read(i_sample, self.n_samples)

        self.board.camera_trigger = camera_pulses

Here there's a thing that's not totally clear to me:
The way the values are written on the piezo (self.board.z_piezo = self.z_waveform.values(self.shifted_time)) and then read to get the wave_part (wave_part = self.recorded_signal.read(i_sample, self.n_samples))

I'm not fully sure of how this two lines relate to each other.

What I found so far is that adding an arbitrary offset to the wavepart seems to get closer to fix the problem,
i.e.
wave_part = self.recorded_signal.read(i_sample, self.n_samples) + 10

Furthermore, the way the waveform for the piezo is generated seems totally correct to me.

Speculation

An hypothesis that I have is that we may need to readjust the minimum/maximum piezo values to account for the part of the wave which is declining towards the minimum. (I haven't manage to test this yet)

Help

I would appreciate any input and clarification in the functioning of the volumetric mode, since maybe I'm missing something that could cause the issue in question.

I'll try to update this thread with what I find next week.

Let me know if you have any ideas regarding this @vilim, @diegoasua and, @vigji

@fedem-p fedem-p self-assigned this Jul 10, 2022
@fedem-p fedem-p added bug Something isn't working help wanted Extra attention is needed labels Jul 10, 2022
@diegoasua
Copy link
Member

diegoasua commented Jul 11, 2022

This is a long known and very annoying bug. Let's try to debug from the basics. Why would it change focus only when switching to Volumetric? Is it because calibration parameters are stored and referenced in different classes within State? Why are these overwritten/switched to default? Can you print the values for calibration when the switching occurs and see if they are the default ones or else the ones set during calibration? I suspect maybe one of the GUI functions that resets stuff at the end of experiment or else at switching mode might be indirectly reloading the calibration parameters. Pinning @vilim 👀

@fedem-p
Copy link
Member Author

fedem-p commented Jul 12, 2022

Thank you @diegoasua for your suggestion and help!
I just checked and, unfortunately, the problem doesn't seem to come from the calibration points, since they don't change between the modes.

The next thing i'll try to do is to compare the
self.board.z_piezo = self.z_waveform.values(self.shifted_time)
with the
wave_part = self.recorded_signal.read(i_sample, self.n_samples)

I suspect part of the difference in the focus could be due to a mismatch in these two parameters, but it's just a supposition (based also on the fact that most other variables either do not change/impact the actual result or they impact it in ways which don't seem relatable to the actual problem, as far as for my testing)

@diegoasua
Copy link
Member

That's a good place to start. I'd also follow the tracks of pressing the planar tab in GUI to all the things it does downstream. Which can be a lot. But yeah, the scanning buffers seem really suspicious.

@fedem-p
Copy link
Member Author

fedem-p commented Jul 13, 2022

So, the comparison between the wave part and the piezo values seem to explain the problem -> even though I'm not sure where to fix and what causes it.

Here's the image comparing the two:

piezo_vs_wave

As you can see the actual values written in the piezo are correct, while the wave_part is offset of 11 up.
I'm not sure why it happens since the piezo range is: "piezo_scan_range": [180.0, 240.0]

Since the wave part is used to calculate the sync for the lateral and frontal beams I'd say it's the root cause of the defocusing.

Now the problems remains to understand how and why this offset is added to the wave.

@vigji
Copy link
Member

vigji commented Jul 13, 2022

is this number independent on the specified scanning range/the piezo positions used for the calibration?

@fedem-p
Copy link
Member Author

fedem-p commented Jul 13, 2022

I've updated the previous image since there was an error (the offset is 11 not 21)

I've also performed other tests (all were done with the same calibration points):

Offset Frequency Range
13.250676215226937 2Hz 180 - 240
11.174464509623384 4Hz 180 - 240
7.022533218384731 8Hz 180 - 240
11.389338789418499 4Hz 160 - 220
10.050374010262717 4Hz 160 - 240
8.746211856660466 4Hz 160 - 260

So this offset is dependent of both the range and frequency.

Update:
Calibration and number of planes don't seem to impact this offset meaningfully

@diegoasua
Copy link
Member

Could this be a timing issue? I/O to the NI card not fast enough and that causes the offset? I can't think of anything else. Scanning buffers are accelerated with Numba JIT compilation. That being said there are known speed limits to python when running on edge even if jitted. @vigji do you agree?

@vigji
Copy link
Member

vigji commented Jul 14, 2022

@fedem-p what happens with constant frequency and piezo range but different offsets?
@diegoasua this seems very regular behavior, and is constant during the acquisition, so I don't think it has something to do with timing.

@fedem-p
Copy link
Member Author

fedem-p commented Jul 14, 2022

This is the results with constant range but different start/end positions for the scanning:

Offset Range Frequency
12.636439993507157 160 - 200 4Hz
12.466397094951128 180 - 220 4Hz
12.313126170159755 200 - 240 4Hz
12.10139356179731 220 - 260 4Hz

Is this what you meant @vigji ?

@diegoasua
Copy link
Member

It still changes a lot even with all conditions being the same except the starting angle of the galvos. That's 0.5 offset change between conditions. To me this smells even more like hardware + timing, nothing to do with the code itself (other than inherent speed of Python)

@vigji
Copy link
Member

vigji commented Jul 14, 2022

This is the results with constant range but different start/end positions for the scanning:

Offset Range Frequency
12.636439993507157 160 - 200 4Hz
12.466397094951128 180 - 220 4Hz
12.313126170159755 200 - 240 4Hz
12.10139356179731 220 - 260 4Hz
Is this what you meant @vigji ?

Can you make even larger changes? Like, 0-40 or 360-400?

@fedem-p
Copy link
Member Author

fedem-p commented Jul 15, 2022

Updated range:

Offset Range Frequency
13.962465948491518 0 - 40 4Hz
12.636439993507157 160 - 200 4Hz
12.466397094951128 180 - 220 4Hz
12.313126170159755 200 - 240 4Hz
12.10139356179731 220 - 260 4Hz
10.754841967555953 360 - 400 4Hz

@vigji
Copy link
Member

vigji commented Jul 15, 2022

So, the only reassuring thing I'm seeing is that it is very linear over all quantities (frequency, range, scanning starting point). I think this makes the hardware + timing hp unlikely, @diegoasua. Although I'm still very puzzled, as this is so linear on all dimensions maybe we can just try to calculate a parameter-dependent offset. Incredibly annoying and ugly, but maybe as we do it we realise what the underlying problem could be :)

@vilim
Copy link
Member

vilim commented Jul 15, 2022

I'll have a more detailed look on the weekend

@fedem-p
Copy link
Member Author

fedem-p commented Jul 15, 2022

Haven't found a smart way to combine all the parameters in order to calculate the offset, but I have this other (equally not ideal) solution:

We can actually compute the shift at run time -> at the moment it slows down the code by roughly 1ms
A better implementation would compute this offset only if the parameters are changed so that it won't introduce any delays in the code.

Here's the code:

wave_part = self.recorded_signal.read(i_sample, self.n_samples) 
off = np.mean((np.array(wave_part[4000:8000]) - np.array(temp1[4000:8000])))
wave_part = wave_part - off

The main problem with this solution are the hard-coded values -> the goal would be to find the part of the wave which is actually parallel with the piezo expected value.
This should be not too complicated to implement but before spending time on it I wanted to hear your opinions on it.

@vilim
Copy link
Member

vilim commented Jul 17, 2022

So my diagnosis is as follows:

For the planar mode and calibration the set piezo position, i.e. the values written to the NI card is used. In the volume mode, the read piezo position is used, and by the interface in (ni.NIBoards) assumes there is a simple relation between the two, just a linear scaling of voltage by self.conf["piezo"]["scale"]. However, I suspect now these two values are out of sync: if you set e.g the piezo to 0 you get 10, and this is not due to the software or the NI board, but the piezo controller/position reading hardware. I think this could be fixed by calibrating the piezo - the setter and the getter of the z_piezo property of NIBoards will become an affine function, the setter the inverse of the getter. In the steady state it is most likely a simple offset. That is one of the dangers of the property-based interface of NIBoards, in that it hides that in reality the board.z_piezo = x is a compeltely different and basically independent process of x = board.z_piezo, which can be simply verified when the power is off: you can set the .z_piezo to anything but the read value will be whatever position the piezo was left at.

I think the piezo controller might even have a hardware knob to adjust this directly (which you could then fix live with an oscilloscope), but I might also be mistaken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants