twVolc / PyCamPermanent

Permanent PiCam (SO2) installation project software
GNU General Public License v3.0
2 stars 2 forks source link

Fixing FOV - not certain radius is in correct units #43

Closed twVolc closed 10 months ago

twVolc commented 1 year ago

When fixing the FOV of the spectrometer (instead of calculating it by correlating with the optical-depth image stack), I'm not certain that the radius works correctly - i.e. is fixing the radius reproducing exactly the same results as if pyplis calculated the radius. I have a feeling it's possible pyplis units for radius are different to the GUI's (e.g. degrees vs pixels) so we might be causing errors if using the fixed radius option.

To test we would need to compare the optical depth time series for the fixed FOV and for the calibrated/correlated FOV (that uses the pyplis FOV search function). They should be identical if all other processing remains the same.

tpering commented 1 year ago

During real-time processing, this will need to be fixed, i.e., we don't want to spend computational time searching for a FOV.

twVolc commented 1 year ago

Details on how the FOV is initially generated (and therefore how the parameters are stored) can be found in PyplisWorker.doas_fov_search line 1955 in master branch. I believe the FOV data are stored in the pyplis DoasCalibData object (in particular probably DoasCalibData.fov so it should be as simple as editing specific attributes in there. I think my main current concern is I didn't adequately test this and I think the pycam GUI's FOV definitions (specifically the radius units) might not correctly map to the pyplis FOV units - this is really what needs checking. Note I've only allowed circular FOV definitions - pyplis does allow eliptical FOVs but I think it's unlikely to make a huge difference to the results considering all the other uncertainties involved so sticking to the simpler circular FOV is fine.

twVolc commented 1 year ago

Sorry, to sumarise this more succinctly - basically what you should be able to do is set the exact same FOV pixel coordinates and radius as found using the FOV search and get exactly the same calibration data (and thus emission rate time series) out as a result.

twVolc commented 11 months ago

I'm not sure the code actually even runs when fixing the FOV at the moment. So this might need more than just getting units right

ubdbra001 commented 11 months ago

Okay, I ran the 3 different FOV/Calibration options:

  1. DOAS FOV search
  2. Fixed FOV
  3. Calibration loaded

I found that 1 and 3 produce almost identical results (as reported before, for some reason 3 has a couple more time points, but outside of these the values are identical), but 2 is on average approx 1.648 different (this is for _phi in flow_raw, not certain what the units are, kg/s?), with a range of 0.165-7.934. Is this a reasonable difference? Or should I poke around with it a bit more?

twVolc commented 11 months ago

That sounds like a pretty significant different - as in 1.648 times larger? I would guess it might be the radius that is getting incorrectly defined, as the centres coordinate should be really simple (I hope), it's just the x and y positions of the FOV.

Rather than comparing the emission rates (although this is totally valid as it should by extension match up) it might be worth comparing the optical depths extracted from the camera images - that are saved in the full calibration file. These should be identical if the fixed FOV is accurately recreating what the FOV search finds (but we'll give a little leeway for the fact there's a chance the FOV search could find an eliptical shape FOV, I think, whereas the fixed FOV we are always just defining a circular FOV.

I think the units are g/s in the saved files, unless we have edited this? It's kg/s in the plots - there's an issue #36 related to the fact it saves in g/s rather than kg/s, but maybe this has been fixed?

twVolc commented 11 months ago

with a range of 0.165-7.934.

I should say that if you're getting emission rates like this then we've almost certainly already shifted to kg/s

ubdbra001 commented 11 months ago

No, 1.648 is the average difference. So for 1. the average value for emmission rate is $X$ and for 2 the average is $X -1.648$, but the differences between the values are not stable (as shown by the range of differences).

Okay I'll have a look at the optical depths. Is there a way of knowing from the output if the FOV from the search is elliptical or not?

I haven't done any work on #36 so I suspect it is the latter

I should say that if you're getting emission rates like this then we've almost certainly already shifted to kg/s

Bear in mind that this is the difference between the two series, rather than the raw emission rate (which tend to be in the 100s or 1000s)

tpering commented 11 months ago

It might be helpful to compare the two emission rates from (1) and (2) directly side by side?

On Thu, Dec 7, 2023 at 11:30 AM Dan Brady @.***> wrote:

No, 1.648 is the average difference. So for 1. the average value for emmission rate is $X$ and for 2 the average is $X -1.648$, but the differences between the values are not stable (as shown by the range of differences).

Okay I'll have a look at the optical depths. Is there a way of knowing from the output if the FOV from the search is elliptical or not?

I haven't done any work on #36 https://github.com/twVolc/PyCamPermanent/issues/36 so I suspect it is the latter

I should say that if you're getting emission rates like this then we've almost certainly already shifted to kg/s

Bear in mind that this is the difference between the two series, rather than the raw emission rate (which tend to be in the 100s or 1000s)

— Reply to this email directly, view it on GitHub https://github.com/twVolc/PyCamPermanent/issues/43#issuecomment-1845176801, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB36N26NLN26RBBBFDUZYILYIGSFBAVCNFSM6AAAAAAZ67IQ66VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBVGE3TMOBQGE . You are receiving this because you commented.Message ID: @.***>

twVolc commented 11 months ago

Bear in mind that this is the difference between the two series, rather than the raw emission rate (which tend to be in the 100s or 1000s)

Ok yeh if the raw values are in the 100s/1000s then it's still in g/s, in which case the relative difference is pretty tiny - like 0.1-1%? This isn't horrendous, but I think it would be nice to look a little deeper to be slightly more confident things are working properly before we merge anything.

There's no way in the files to work out if the FOV is elliptical or not - where we are now is about the level of knowledge I got to with how the pyplis fov class works. Essentially what we want is to be able to perfectly replicate PyplisWorker.calib_pears.fov which is a pyplis doascalib.DoasFOV object. That object isn't actually crazily complicated, so one thing to do may be to run processing with the FOV search, then pause in the debugger and check what that calib_pears.fov object looks like - hopefully the coordinates and radius can be identified there.

I've just double-checked the pyplis paper and I'm pretty confident that actually the pearson method of FOV search (which is what we use) can only generate circular FOVs - it's the more complex IFR method that can generate an ellipse. So actually I think if we get the radius and centre coordinates correct, we should be able to get identical optical depths out (assuming the same ambient backgroun region and background model are selected of course - it may be easiest to use BG_mode=7 for these tests)

ubdbra001 commented 11 months ago

It might be helpful to compare the two emission rates from (1) and (2) directly side by side?

Here's a sample of the emission rates data (Happy to post the full file if you think it would be useful)

image

This is for the flow_raw, _phi_x is (1) and _phi_y is (2). The rest of the columns (_velo_eff, etc) have identical values for the two options.

twVolc commented 11 months ago

Yeh a very tiny difference, but I would feel more comfortable if we could get the optical depths identical before merging this, as that should be possible if we are truly reproducing the FOV. I think it the radius size that is almost certainly the issue

ubdbra001 commented 11 months ago

Okay, I will continue digging!

ubdbra001 commented 11 months ago

There's no way in the files to work out if the FOV is elliptical or not - where we are now is about the level of knowledge I got to with how the pyplis fov class works. Essentially what we want is to be able to perfectly replicate PyplisWorker.calib_pears.fov which is a pyplis doascalib.DoasFOV object. That object isn't actually crazily complicated, so one thing to do may be to run processing with the FOV search, then pause in the debugger and check what that calib_pears.fov object looks like - hopefully the coordinates and radius can be identified there.

I've just double-checked the pyplis paper and I'm pretty confident that actually the pearson method of FOV search (which is what we use) can only generate circular FOVs - it's the more complex IFR method that can generate an ellipse. So actually I think if we get the radius and centre coordinates correct, we should be able to get identical optical depths out (assuming the same ambient backgroun region and background model are selected of course - it may be easiest to use BG_mode=7 for these tests)

Would this bit of code (specifically line 2266) in pyplis_worker constrain the FOV to be circular? https://github.com/twVolc/PyCamPermanent/blob/4edfb0ca0b651f07c21951ce248f6114d62a5060/pycam/so2_camera_processor.py#L2262-L2270

twVolc commented 11 months ago

That's a good point - I'm guessing I've taken that straight from a pyplis example code, so I guess this does constrain it to being circular. Line 2267 is then where the search happens so after that line the fov attribute should be in its final state. I'm not certain what line 2271 does but this may affect things too: self.doas_fov_extent = self.calib_pears.fov.pixel_extend(abs_coords=True)

twVolc commented 11 months ago

self.doas_fov_extent = self.calib_pears.fov.pixel_extend(abs_coords=True)

Actually I think this just returns the coordinates of the extent of the FOV, so it doesn't change anything. Basically I guess the aim is to get the output of this extent to be exactly the same as if we fix the FOV

twVolc commented 11 months ago

self.doas_fov_extent = self.calib_pears.fov.pixel_extend(abs_coords=True)

Actually I think this just returns the coordinates of the extent of the FOV, so it doesn't change anything. Basically I guess the aim is to get the output of this extent to be exactly the same as if we fix the FOV

Thinking about it, this is probably the quickest test to get things working. We just need to be able to generate a DoasFOV object where DoasFOV.pixel_extend(abs_coords=True) is identical to the same output after the doas_fov_search. When this matches I'm pretty certain the optical depth time series, and therefore emission rates, will also match.

twVolc commented 11 months ago

Just as extra info: I looked into it a little more and I think the line s.g2dasym = False # Allow only circular FOV (not eliptical) only comes into play if the fov search method='ifr', so even if this were set incorrectly I'm pretty confident we would always have a circular FOV with method='pearson'.

ubdbra001 commented 11 months ago

Okay I had a poke around in the DoasFOV object, and noticed a few differences:

  1. This is the object after the doas_fov_search: image

  2. This is the one created at the start of a fix_fov processing run: image

A few differences between the objects jump out here, any idea if any of these would have an impact?

I've made pkls of these objects if you'd like to inspect them yourself.

twVolc commented 11 months ago

Ok, so the pyrlevel is the amount of subsampling taking place in the image I believe. So pyrlevel=0 means no subsampling - our images have the dimensions 486 x 648. pyrlevel=1 means the object believes that the images are being downsampled to 243 x 324, so when the relative x and y values are being set (cx_rel and cy_rel) pyplis is then converting these to absolute coordinates by projecting those coordinates onto a full resolution image - so x_abs and y_abs are double cx_rel and cy_rel. This is my current thinking anyway. Line 2316 in so2_camera_processor sets this (generate_doas_fov()), and I'm guessing I just incorrectly set pyrlevel thinking that 1 meant no subsampling. If you change this to 0 hopefully it will help things, automatically fixing x_abs and y_abs, and looking at how sigma_x_abs and sigma_y_abs are caluculated I think this will automatically get them correct too (I've got no idea what these parameters do...)

I'm amazed that the emission rates seem to come out so similar if the coordinates are so drastically different, so I wonder if the calibration uses cx_rel and cy_rel rather than x_abs and y_abs, so you may find this changes absolutely nothing in the output, but definitely worth getting this right - maybe the sigmas have a small affect that could be accounting for our slight differences...

twVolc commented 11 months ago

Could also try setting the camera object, just in case. I think PyplisWorker.cam is the right thing, so just setting the DoasFOV.camera object to that within the generate_doas_fov() function should hopefully do the trick.

I'm slightly confused as to why it doesn't have a camera object though - in generate_dos_fov(), self.cam is passed to DoasCalibData on object instantiation. Looking at that object's __init__ that camera object should be passed to DoasFOV which is created as an attribute of DoasCalibData, if fov=None which it should do. I can't work out why this isn't happening.

ubdbra001 commented 11 months ago

Ok, so the pyrlevel is the amount of subsampling taking place in the image I believe. So pyrlevel=0 means no subsampling - our images have the dimensions 486 x 648. pyrlevel=1 means the object believes that the images are being downsampled to 243 x 324, so when the relative x and y values are being set (cx_rel and cy_rel) pyplis is then converting these to absolute coordinates by projecting those coordinates onto a full resolution image - so x_abs and y_abs are double cx_rel and cy_rel. This is my current thinking anyway. Line 2316 in so2_camera_processor sets this (generate_doas_fov()), and I'm guessing I just incorrectly set pyrlevel thinking that 1 meant no subsampling. If you change this to 0 hopefully it will help things, automatically fixing x_abs and y_abs, and looking at how sigma_x_abs and sigma_y_abs are caluculated I think this will automatically get them correct too (I've got no idea what these parameters do...)

I'm amazed that the emission rates seem to come out so similar if the coordinates are so drastically different, so I wonder if the calibration uses cx_rel and cy_rel rather than x_abs and y_abs, so you may find this changes absolutely nothing in the output, but definitely worth getting this right - maybe the sigmas have a small affect that could be accounting for our slight differences...

I changed pyrlevel to 0 and it did result in the abs and sigmas matching the values for (1), but it had no effect on the final output, so it's not that.

Could also try setting the camera object, just in case. I think PyplisWorker.cam is the right thing, so just setting the DoasFOV.camera object to that within the generate_doas_fov() function should hopefully do the trick.

I'm slightly confused as to why it doesn't have a camera object though - in generate_dos_fov(), self.cam is passed to DoasCalibData on object instantiation. Looking at that object's __init__ that camera object should be passed to DoasFOV which is created as an attribute of DoasCalibData, if fov=None which it should do. I can't work out why this isn't happening.

So for fix_fov you are correct the camera is set, it's not set for the doas_fov_search. I'm going to try unsetting it for fix_fov to see if it makes a difference.

ubdbra001 commented 11 months ago

Another thought I had after looking at the calibration data is that the weird thing where some timepoints are skipped/not skipped may play a role, by slightly altering the regression coefficients for the two calibration methods.

Here is a snapshot of the full_calibration files side by side (left side is for doas_fov_search and right is for fix_fov: image

They are identical until row 356, and then there are two differences:

My understanding of the calibration is that it uses the correlation coefficients to adjust the pixel values of the image, and then the emission rate is estimated from these adjusted values. So, if the data being used for the correlation is slightly different between the two calibration approaches then the coefficients will be different, and the resulting adjustments and emission rates will be different. Is my grasp of that correct?

twVolc commented 11 months ago

So for fix_fov you are correct the camera is set, it's not set for the doas_fov_search. I'm going to try unsetting it to see if it makes a difference.

Ah, that's probably why it always throws a warning saying no camera is set at the start of the FOV search. I wonder if we should work out why it isn't set and set it. Again, it probably won't make a difference, so it's slightly confusing that we aren't getting exactly the same results.

I think checking fov_extend might be important, as I think this defines the extent of the fov. In so2_camera_processor there's 2 important places:

twVolc commented 11 months ago

Another thought I had after looking at the calibration data is that the weird thing where some timepoints are skipped/not skipped may play a role, by slightly altering the regression coefficients for the two calibration methods.

Here is a snapshot of the full_calibration files side by side (left side is for doas_fov_search and right is for fix_fov: image

They are identical until row 356, and then there are two differences:

  • The coefficients from the regression are included earlier in doas_fov_search (they should be going from the same time right?)
  • There are two missing timepoints in doas_fov_search, 16:35:10 and 16:35:15

My understanding of the calibration is that it uses the correlation coefficients to adjsut the pixel values of the image, and then the emission rate is estimated from these adjusted values. So, if the data being used for the correlation is slightly different between the two calibration approaches then the coefficients will be different, and the resulting adjustments and emission rates will be different. Is my grasp of that correct?

Ah yes, this will be the problem then! So the DOAS FOV is being correctly defined, which is good. But for some reason we are missing some data points when using the DOAS FOV search so that puts everything out ever so slightly? So actually the fixed FOV is working better than the search now... am I right in thinking this relates to another issue that you've raised a few times?

ubdbra001 commented 11 months ago

Possibly, though I'm not entirely sure how. I've made some notes in #73, but effectively: when using the Preloaded calibration option, there are 3 additional time points included in the emission rate data. Two of which correspond to the points being dropped here, which will be around when the doas_fov_search is run.

twVolc commented 11 months ago

I'd guess it's something simple to do with indexing when looping back through the old data once the FOV has been found, but then why it's new data 5 minutes later that's dropped I have no idea. I'm not quite sure how they then turn up in the preloaded calibration though as that suggests they must have been calculated and saved in the array in the first place?

twVolc commented 11 months ago

Either way, I think this means we can close this issue when any of your updates are merged, as this isn't a fixing FOV issue?

It would be nice to double check that fixing FOV in the config file correctly loads that on start-up (including updating the DOAS-camera calibration frame with the pixel coordinate values). So that this can be run straight from loading a config - perhaps this is what you're doing already so it already works?

ubdbra001 commented 11 months ago

Either way, I think this means we can close this issue when any of your updates are merged, as this isn't a fixing FOV issue?

It would be nice to double check that fixing FOV in the config file correctly loads that on start-up (including updating the DOAS-camera calibration frame with the pixel coordinate values). So that this can be run straight from loading a config - perhaps this is what you're doing already so it already works?

Currently I haven't added anything to the config to do with this, but that should be reasonably straightforward to do.

Closing the issue is fine by me, but I think it would be worth opening a new issue describing the discrepancies between the doas_fov_search and fix_fov calibrations (and reference the discussion here), as we'll likely come back to it.

ubdbra001 commented 11 months ago

Ah, that's probably why it always throws a warning saying no camera is set at the start of the FOV search. I wonder if we should work out why it isn't set and set it. Again, it probably won't make a difference, so it's slightly confusing that we aren't getting exactly the same results.

As predicted, this doesn't make a difference.

ubdbra001 commented 11 months ago

Possibly silly question, but I'm assuming that you'd like the fix_fov params values (e.g. x, y, rad) generated by doas_fov_search to be included in the output config? GIven that the config file is produced at the start of the run, would it be worth updating the config as soon as doas_fov_search has finished?

twVolc commented 11 months ago

Yeh that would be great, and yes it seems like a sensible time to update it right after the FOV search has successfully found the coordinates.

In terms of loading things back in, it looks like some of the variables currently aren't in DOASFOVSearchFrame.vars and also they don't have the correct names relative to PyplisWorker. So I guess there might be a bit of thinking/editing there, but presumably it's the same as the stuff you've previously done when setting up the config files in the first place.