Ubdbra001/issue18

[ubdbra001]

Current approach is to loading the config file to pyplis_worker, which saves it in an attribute as a dictionary (pyplis_worker.config). This is then read in by the GUI elements when required (currently only once at startup to load the deafult config). Still needs:

• [x] Switch default config to yaml file
• [x] Loading in user defined config
  • [x] Ensure that GUI is updated with new config
• [x] Ensuring user changes in the GUI are reflected in the pyplis_worker.config dict
• [x] Writing the dict on start/finish of the run

There are probably other bits I'm forgetting too.

Edit: Need to:

• [x] Save img registration in Processing dir
• [x] Move default_cam_geom path into config file
  • [x] Save default_cam_geom file alongside config
• [x] pull & merge current dev branch

[ubdbra001]

@twVolc I think this is pretty much there. I'm just trying to add saving the cam_geom file alongside the config, but I think it's worth you taking a look at it now to see if there's anything I've missed.

[ubdbra001]

Looks like the Optical flow settings aren't saving properly yet.

Edit: Okay, should be fixed now. @twVolc if you're testing this make sure to pull the branch to get the most recent version.

[twVolc]

I'm just doing some tests. Background modelling doesn't seem to be working properly. I've loaded in a sequence, then loaded a CP registration (which also actually doesn't update until you click No Registration then back to CP registration, but this might be a bug in the main code not this specific branch - I need to test). When I change the ambient ROI in the analysis window this doesn't actually change the background modelling - either automatically (which it should and previously has done) but also even in the Background Model window when clicking Run. When the BG mode is 7, this ambient rectangle should be changing the output - essentially the image is normalised to this region, so if you draw the ROI over the plume the optical depths should decrease massively, and if you draw it in the clear sky optical depths of the plume should be positive. It seems the ambient ROI isn't being correctly updated in the config file.

[ubdbra001]

I'm just doing some tests. Background modelling doesn't seem to be working properly. I've loaded in a sequence, then loaded a CP registration (which also actually doesn't update until you click No Registration then back to CP registration, but this might be a bug in the main code not this specific branch - I need to test). When I change the ambient ROI in the analysis window this doesn't actually change the background modelling - either automatically (which it should and previously has done) but also even in the Background Model window when clicking Run. When the BG mode is 7, this ambient rectangle should be changing the output - essentially the image is normalised to this region, so if you draw the ROI over the plume the optical depths should decrease massively, and if you draw it in the clear sky optical depths of the plume should be positive. It seems the ambient ROI isn't being correctly updated in the config file.

I think there is something weird going on with the bg model updating part of the GUI in general. It wasn't updating when I loaded in the config, and needed another run to make it work. Judging by the results I currently thin that it's more cosmetic than anything else (i.e. the final results are the same), but it is still annoying nonetheless. I spent some time looking into it last week but didn't make any progress, but it is right at the top of my to-do list.

[twVolc]

Do the results change if you draw the ROI in a totally different area (e.g. over the plume)? That would prove it's the GUI not updating rather than the model. But it is very useful to be able to redraw the ROI in the analysis window and have it automatically update the SO2 image

[ubdbra001]

I haven't tried that, but I'll add it to the things to test when I get back to it.

[ubdbra001]

Okay, can confirm that chanaging the ROI on the SO2 image plot does update the underlying ROI value (in pyplis) and the subsequest results it produces. So, looks like it is just a graphical thing. Completely appreciate that it is a useful feature to have though, so will start looking into what the underlying issue is early next week.

[twVolc]

What are your current thoughts on how best to set defaults? Is the plan to have a button that can set all current settings as the default? Or perhaps a way of selecting a specific config file to become the default? In both cases I guess it involves pulling all the other files along with the yml file - i.e. just dragging the yml file you want into pycam/conf and renaming it processing_setting_defaults.yml wouldn't work unless the other files are also copied over to that folder.

This is all probably desirable rather than essential prior to the merge, but I think we discussed that it would be nice to have a button that takes all of the current settings and sets them as the default for start-up. Has that been implemented somewhere?

[twVolc]

Just been testing things. Loading a new config through File > Load > Load config file doesn't seem to update the camera geometry with the new configuration (in cam_geom.txt)

[ubdbra001]

Just been testing things. Loading a new config through File > Load > Load config file doesn't seem to update the camera geometry with the new configuration (in cam_geom.txt)

Will look into it. Thanks for letting me know.

[twVolc]

What are your current thoughts on how best to set defaults? Is the plan to have a button that can set all current settings as the default? Or perhaps a way of selecting a specific config file to become the default? In both cases I guess it involves pulling all the other files along with the yml file - i.e. just dragging the yml file you want into pycam/conf and renaming it processing_setting_defaults.yml wouldn't work unless the other files are also copied over to that folder.

This is all probably desirable rather than essential prior to the merge, but I think we discussed that it would be nice to have a button that takes all of the current settings and sets them as the default for start-up. Has that been implemented somewhere?

Ignore this, just seen it's already an issue so isn't needed before the merge. My memory...

[ubdbra001]

Just been testing things. Loading a new config through File > Load > Load config file doesn't seem to update the camera geometry with the new configuration (in cam_geom.txt)

Could you provide a bit more info about what you did? I just had a go with it and didn't see a problem. If you could attach the config and the cam_geom.txt files that would also help.

[twVolc]

Could you provide a bit more info about what you did? I just had a go with it and didn't see a problem. If you could attach the config and the cam_geom.txt files that would also help.

I updated the camera geometry - to a new volcano (I've attached the file, although you may need to first download the reventador infor through the GUI for it to recognise this volcano? This is done useing the Get Info button in the camera geometry configuration pane) cam_geom.txt I then saved all of the settings as a new config file (which included saving the new cam_geom.txt file, and this all looks good). I then restarted things, so that villarrica was back, and tried loading the config. Looks like all other things changed (haven't checked everything yet) but when opening the camera geometry frame again it is stuck on villarrica.

I may well have done something wrong, but I've tried a few times and it doesn't seem to be changing.

[twVolc]

Could you provide a bit more info about what you did? I just had a go with it and didn't see a problem. If you could attach the config and the cam_geom.txt files that would also help.

I updated the camera geometry - to a new volcano (I've attached the file, although you may need to first download the reventador infor through the GUI for it to recognise this volcano? This is done useing the Get Info button in the camera geometry configuration pane) cam_geom.txt I then saved all of the settings as a new config file (which included saving the new cam_geom.txt file, and this all looks good). I then restarted things, so that villarrica was back, and tried loading the config. Looks like all other things changed (haven't checked everything yet) but when opening the camera geometry frame again it is stuck on villarrica.

I may well have done something wrong, but I've tried a few times and it doesn't seem to be changing.

Ah I've just looked at the process_config.yml file - it doesn't change default_cam_geom - it's still set as ./pycam/conf/cam_geom/villarrica_26-03-2018.txt but really this should change to the cam_geom file that is saved alongside all the other config files

[twVolc]

Could you provide a bit more info about what you did? I just had a go with it and didn't see a problem. If you could attach the config and the cam_geom.txt files that would also help.

I updated the camera geometry - to a new volcano (I've attached the file, although you may need to first download the reventador infor through the GUI for it to recognise this volcano? This is done useing the Get Info button in the camera geometry configuration pane) cam_geom.txt I then saved all of the settings as a new config file (which included saving the new cam_geom.txt file, and this all looks good). I then restarted things, so that villarrica was back, and tried loading the config. Looks like all other things changed (haven't checked everything yet) but when opening the camera geometry frame again it is stuck on villarrica. I may well have done something wrong, but I've tried a few times and it doesn't seem to be changing.

Ah I've just looked at the process_config.yml file - it doesn't change default_cam_geom - it's still set as ./pycam/conf/cam_geom/villarrica_26-03-2018.txt but really this should change to the cam_geom file that is saved alongside all the other config files

I'm getting the same issue with ILS_path - this should track updates from View > More Windows > DOAS calibration > Load ILS but new paths aren't saved in the config file

[ubdbra001]

Ah I've just looked at the process_config.yml file - it doesn't change default_cam_geom - it's still set as ./pycam/conf/cam_geom/villarrica_26-03-2018.txt but really this should change to the cam_geom file that is saved alongside all the other config files

Looks like the save config doesn't save the new cam_geom location. I'll have a look into that tomorrow. Edit: Was just a single line of code. FIxed now.

[twVolc]

Could you provide a bit more info about what you did? I just had a go with it and didn't see a problem. If you could attach the config and the cam_geom.txt files that would also help.

I updated the camera geometry - to a new volcano (I've attached the file, although you may need to first download the reventador infor through the GUI for it to recognise this volcano? This is done useing the Get Info button in the camera geometry configuration pane) cam_geom.txt I then saved all of the settings as a new config file (which included saving the new cam_geom.txt file, and this all looks good). I then restarted things, so that villarrica was back, and tried loading the config. Looks like all other things changed (haven't checked everything yet) but when opening the camera geometry frame again it is stuck on villarrica. I may well have done something wrong, but I've tried a few times and it doesn't seem to be changing.

Ah I've just looked at the process_config.yml file - it doesn't change default_cam_geom - it's still set as ./pycam/conf/cam_geom/villarrica_26-03-2018.txt but really this should change to the cam_geom file that is saved alongside all the other config files

I'm getting the same issue with ILS_path - this should track updates from View > More Windows > DOAS calibration > Load ILS but new paths aren't saved in the config file

spec_dir and dark_spec_dir also don't seem to change in the config file. Keys that aren't changing when saving a config file:

• default_cam_geom
• ILS_path
• spec_dir
• dark_spec_dir

I haven't checked everyone, so there may be more. I guess to be thorough we just need to edit each parameter and be certain it changes when being saved.

[ubdbra001]

spec_dir and dark_spec_dir also don't seem to change in the config file. Keys that aren't changing when saving a config file:

* `default_cam_geom`

* `ILS_path`

* `spec_dir`

* `dark_spec_dir`

I haven't checked everyone, so there may be more. I guess to be thorough we just need to edit each parameter and be certain it changes when being saved.

Aside from default_cam_geom the rest are associated with Doas/Ifit, which I suspect is the problem. Will save looking into that until tomorrow.

[twVolc]

Aside from default_cam_geom the rest are associated with Doas/Ifit, which I suspect is the problem. Will save looking into that until tomorrow.

Great! It would be nice to have this working for the DOAS/IFit side of things too as I envisage incorporating new variables into the config for IFit settings in the not-too-distant future - such as start_fit_wave and end_fit_wave (not for this merge don't worry!)

[ubdbra001]

Aside from default_cam_geom the rest are associated with Doas/Ifit, which I suspect is the problem. Will save looking into that until tomorrow.

Great! It would be nice to have this working for the DOAS/IFit side of things too as I envisage incorporating new variables into the config for IFit settings in the not-too-distant future - such as start_fit_wave and end_fit_wave (not for this merge don't worry!)

So, I'm thinking there are two ways to approach this:

1. The quick way - Write a function where a list of the doas params are hardcoded, and then when the config file is saved these values are collected from doas_worker and the config dict in pyplis_worker is updated. The downside to this is that this list will have to be updated everytime a new doas param is added.
2. The more thorough way - Set up an intermediate dict in doas_worker (like I've done with pyplis_worker) that can then be passed back to pyplis_worker when it comes time to save the config. The downside to this is that it will take some time to go through the code and ensure that this works correctly (especially as I have a poorer grasp of the internals of doas_worker than I do pyplis_worker).

I'm more inlcined to go for 1 here, as it doesn't rule out doing 2 later and this is already a bit of a hefty PR. What do you think?

[twVolc]

So, I'm thinking there are two ways to approach this:

1. The quick way - Write a function where a list of the doas params are hardcoded, and then when the config file is saved these values are collected from doas_worker and the config dict in pyplis_worker is updated. The downside to this is that this list will have to be updated everytime a new doas param is added.
2. The more thorough way - Set up an intermediate dict in doas_worker (like I've done with pyplis_worker) that can then be passed back to pyplis_worker when it comes time to save the config. The downside to this is that it will take some time to go through the code and ensure that this works correctly (especially as I have a poorer grasp of the internals of doas_worker than I do pyplis_worker).

I'm more inlcined to go for 1 here, as it doesn't rule out doing 2 later and this is already a bit of a hefty PR. What do you think?

Yeh I think 1 sounds fine to me, I don't think we should embark on anything substantial regarding this right now. It would just be good to get this side of things working. Updating that list to add new params later doesn't sound too arduous.

The main thing is - as the majority of these DOAS params currently are paths - just ensuring that things are correctly loaded back up any time a new config file is loaded in.

[ubdbra001]

Yeh I think 1 sounds fine to me, I don't think we should embark on anything substantial regarding this right now. It would just be good to get this side of things working. Updating that list to add new params later doesn't sound too arduous.

Great, I'll do that today. It's mostly the fact that you need to remember to update the list that makes 1. a slightly worse solution. I don't know how you are, but for me it's easy to forget to update it and then spend ages trying to work out what's going wrong.

[twVolc]

Yeh I think 1 sounds fine to me, I don't think we should embark on anything substantial regarding this right now. It would just be good to get this side of things working. Updating that list to add new params later doesn't sound too arduous.

Great, I'll do that today. It's mostly the fact that you need to remember to update the list that makes 1. a slightly worse solution. I don't know how you are, but for me it's easy to forget to update it and then spend ages trying to work out what's going wrong.

Yeh I know what you mean. If you just signpost the list as well as possible (e.g. a comment at the top of the config setup or somewhere you find suitable) hopefully if/when anyone comes to make updates they head for the config section and see how to implement it.

[ubdbra001]

Okay, those two commits should ensure the last params related to doas/ifit are are updated