search

twVolc / PyCamPermanent

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

Preloaded calibration in config doesn't change correctly #105

Closed twVolc closed 9 months ago

twVolc commented 9 months ago

If preloaded is set as the initial config setting (when loading a config file), and the calibration file is loaded, changing to a different calibration (e.g. DOAS) via GUI settings doesn't actually change things - it is still calibrated by the preloaded data, or at least the preloaded calibration is still the one that is saved. I think it might actually be the latter, as I'm able to open the camera-DOAS calibration window and it looks like that has been running, so that bit maybe is working, but for some reason the config file still saves cal_series_path and the calibration series is definitely a copy of the preloaded series as I have tested this with a series from a different time - the full_calibration contains the data from the incorrect period of time related to the preloaded data.

ubdbra001 commented 9 months ago

I'm pretty certain that if you change the calibration type from Pre-loaded to something else the data stored in the calibration_series attribute is retained but not used as part of processing. But it sounds like the fact it is retained is causing problems later on.

ubdbra001 commented 9 months ago

Ah, I've just spotted what's happening: https://github.com/twVolc/PyCamPermanent/blob/f81ec766676f25c24874f63d31bc86c63eefb581/pycam/so2_camera_processor.py#L3919-L3920

When saving it is only checking to see if the series exists, and nothing else. So, if it was loaded from the last run then it will just save that one regardless of the calibration type chosen.

There are two ways of dealing with this:

  1. Add a conditional here so that it also checks to make sure the calibration type is "Pre-loaded"
  2. Adjust the code so that on a new run where the calibration type is not "Pre-loaded" it dumps the calibration_series attribute.

Any preference?

twVolc commented 9 months ago

Ah ok I see. I wonder if both might be the sensible option? That would ensure that if any further development is done on this, e.g. allowing commandline stuff outside of the GUI, even if calibration_series randomly got re-generated the save function will still correctly ssave the DOAS data if that's the cal type. But simultaneously, it probably makes sense to do 2, so that if the cal-type isn't preloaded there isn't this random calibration_series attribute which might confuse anyone looking looking more deeply into the object. Does that make sense?

twVolc commented 9 months ago

With the latter, we'd just need to make sure that if you then flip back to cal_type being preloaded in the settings GUI that it relaods the calibration data - but presumably this wouldn't actually require any new code as that calibration series is probably loaded when you click Ok as long as there is a path to the calibration file?

ubdbra001 commented 9 months ago

With the latter, we'd just need to make sure that if you then flip back to cal_type being preloaded in the settings GUI that it relaods the calibration data - but presumably this wouldn't actually require any new code as that calibration series is probably loaded when you click Ok as long as there is a path to the calibration file?

Yep, if the calibration path is set and cal_type is set to preloaded then the cal series at the cal path will be loaded when a user presses either "Ok"

ubdbra001 commented 9 months ago

Ah ok I see. I wonder if both might be the sensible option? That would ensure that if any further development is done on this, e.g. allowing commandline stuff outside of the GUI, even if calibration_series randomly got re-generated the save function will still correctly ssave the DOAS data if that's the cal type. But simultaneously, it probably makes sense to do 2, so that if the cal-type isn't preloaded there isn't this random calibration_series attribute which might confuse anyone looking looking more deeply into the object. Does that make sense?

Yep, that makes sense to me. Any thoughts on when it should be deleted? I was thinking that it could be deleted when cal_type_int is set to something other than "Preloaded", but that would mean it is dropped as soon as the ProcessSettings window is closed (or a new run is started). Does that sound reasonable to you?

twVolc commented 9 months ago

Any thoughts on when it should be deleted? I was thinking that it could be deleted when cal_type_int is set to something other than "Preloaded", but that would mean it is dropped as soon as the ProcessSettings window is closed (or a new run is started). Does that sound reasonable to you?

Yes this makes sense to me, I would go for when Ok is clicked in the ProcessSettings window, but keeping reference to the cal_series file path so it remains there if the user wants to flip back to Preloaded later.