srlabUsask / crhmcode

GNU General Public License v3.0
8 stars 4 forks source link

2-D parameters displayed default values in Shared #408

Open loganxingfang opened 2 years ago

loganxingfang commented 2 years ago

I have noticed some issues regarding the 2 dimensional parameters displaying default values in the Shared when viewing it in Parameters GUI. There 2 dimensional parameters have one dimension for HRU and the other dimension for layer or fixed value of 2. These issues will be demonstrated below using prj files and screen shots.

  1. From the prj file Fortress Mountain Basin test_2013-21_modified_22Mar22a.prj, can be found here Fortress Mountain Basin test_2013-21_modified_22Mar22a.zip

When opening this prj file in current CRHMcode GUI and opening the Parameters GUI, default values were displayed for both parameters obs_elev and soil_withdrawal in Shared in the Parameters GUI, ie. 0 is default for obs_elev and 3 is default for soil_withdrawal. See screen shot here: screen shot 1

However, when opening this prj file in TextPad or other text editor, both parameters obs_elev and soil_withdrawal have their correct values, see screen shots below in the highlighted: screen shot 2 screen shot 3

I think the issue is in the CRHMmain.cpp file used by current CRHMcode: https://github.com/srlabUsask/crhmcode/blob/master/crhmcode/src/core/CRHMmain.cpp

On line 609, the code is still old code, roughly on 6 Feb 2019 (ie. source code CRHM_020619). I noticed this issue back in 2018 and worked on this with Tom in May 2019, and Tom put some change to fix this. From CRHMmain.cpp file in Borland source code: https://github.com/srlabUsask/crhmcode/blob/master/borland_source/CRHMmain.cpp

On line 2736 to 2738, the change Tom put on 15 May 2019 seemed to fix this. I will have the following screen shots to show that for Borland CRHM.

For the same prj file, I opened it in Borland CRHM and its Parameters GUI, both parameters obs_elev and soil_withdrawal were displayed correct values in Shared in Parameters GUI, see screen shots: screen shot 4 screen shot 5

  1. On the other hand, current CRHMcode seems to display other 2 dimensional parameters. I used the prj file WCRB_currentClim_currentVeg_flow - Copy.prj for this demonstration. WCRB_currentClim_currentVeg_flow - Copy.zip

Screen shots below show that when opening this prj file in CRHMcode, both depths and por parameters have 20 layers and 4 HRUs, and their values are correctly displayed in Shared in Parameters GUI compared to them in highlighted when opening this prj file in TextPad: screen shot 6 screen shot 7 screen shot 8

In contrast, when opening this prj file in Borland CRHM, default values were displayed for both depths and por parameters in Shared in Parameters GUI: screen shot 9 screen shot 10

This suggests that the change Tom put in CRHMmain.cpp as mentioned above did not handle this case.

  1. To further examine this issue, I used the prj file ElbowAtYYC_test_mod22Jul2020_with_WRF4km_obs_with_sub_WBvar_estimate_macros.prj ElbowAtYYC_test_mod22Jul2020_with_WRF4km_obs_with_sub_WBvar_estimate_macros.zip

This is the prj file that I set parameters values outside Borland CRHM GUI and saved outside Borland CRHM using other program, thus there is no parameter listed under Shared.

When opening ElbowAtYYC_test_mod22Jul2020_with_WRF4km_obs_with_sub_WBvar_estimate_macros.prj in Borland CRHM, the 2 dimensional parameters such as depths, por, soil_withdrawal displayed correct values in Shared: screen shot 11 screen shot 12 screen shot 13

Then, I saved ElbowAtYYC_test_mod22Jul2020_with_WRF4km_obs_with_sub_WBvar_estimate_macros.prj in Borland CRHM and saved as ElbowAtYYC_test_mod22Jul2020_with_WRF4km_obs_with_sub_WBvar_estimate_macros_BorlandCRHMSaved.prj
ElbowAtYYC_test_mod22Jul2020_with_WRF4km_obs_with_sub_WBvar_estimate_macros_BorlandCRHMSaved.zip

I opened ElbowAtYYC_test_mod22Jul2020_with_WRF4km_obs_with_sub_WBvar_estimate_macros_BorlandCRHMSaved.prj file in Borland CRHM and its Parameters GUI, then default values were displayed for these 2 dimensional parameters such as depths, por, soil_withdrawal: screen shot 14 screen shot 15 screen shot 16

However, when opening ElbowAtYYC_test_mod22Jul2020_with_WRF4km_obs_with_sub_WBvar_estimate_macros_BorlandCRHMSaved.prj file in TextPad, the correct values are shown for these 2 dimensional parameters such as depths, por, soil_withdrawal: screen shot 17 screen shot 18 screen shot 19

  1. I also did the same step 3 in CRHMcode, I used the prj file ElbowAtYYC_test_mod22Jul2020_with_WRF4km_obs_with_sub_WBvar_estimate_macros.prj, when it in CRHMcode Parameters GUI, the 2 dimensional parameters such as depths, por, soil_withdrawal displayed correct values in Shared: screen shot 20

Then, I saved ElbowAtYYC_test_mod22Jul2020_with_WRF4km_obs_with_sub_WBvar_estimate_macros.prj in CRHMcode and saved as ElbowAtYYC_test_mod22Jul2020_with_WRF4km_obs_with_sub_WBvar_estimate_macros_CRHMcodeSaved.prj
ElbowAtYYC_test_mod22Jul2020_with_WRF4km_obs_with_sub_WBvar_estimate_macros_CRHMcodeSaved.zip

I opened ElbowAtYYC_test_mod22Jul2020_with_WRF4km_obs_with_sub_WBvar_estimate_macros_CRHMcodeSaved.prj file in CRHMcode and its Parameters GUI, then default values were displayed for these 2 dimensional parameters such as depths, por, soil_withdrawal: screen shot 21

However, when opening ElbowAtYYC_test_mod22Jul2020_with_WRF4km_obs_with_sub_WBvar_estimate_macros_CRHMcodeSaved.prj file in TextPad, the correct values are shown for these 2 dimensional parameters such as depths, por, soil_withdrawal: screen shot 22 screen shot 23 screen shot 24

I think these issues are linked to code in CRHMmain.cpp file to handle displaying 2 dimensional parameters from Group prj file in the Shared in Parameters GUI, and additional issues are displaying 2 dimensional parameters from Group prj file in the Shared in Parameters GUI either with or without saving prj file.

There needs some additional code to handle above exceptions in current CRHMmain.cpp code for both Borland CRHM and CRHMcode.

loganxingfang commented 2 years ago

Regarding Step 3 and 4, I noticed that all shared parameters regardless of 2 dimensional or not are having their default values displayed in Shared in Parameters GUI of Borland CRHM or CRHMcode after saving, ie. in prj files: ElbowAtYYC_test_mod22Jul2020_with_WRF4km_obs_with_sub_WBvar_estimate_macros_BorlandCRHMSaved.prj or ElbowAtYYC_test_mod22Jul2020_with_WRF4km_obs_with_sub_WBvar_estimate_macros_CRHMcodeSaved.prj

However, these shared parameters regardless of 2 dimensional or not do not have default values displayed when viewing them in TextPad.

I think this could be attributed to there are shared parameters with different numbers of HRU; e.g. for parameters with HRU dimension, JCrop_Harvest, JCrop_Start and so have 7 HRUs that are shared among groups such as GrpS and GrpU, while the majority of shared parameters have14 HRUs that are shared among groups such as GrpJ, GrpK, GrpN, and so on.

For prj file Fortress Mountain Basin test_2013-21_modified_22Mar22a.prj, it was also saved from another prj file Fortress Mountain Basin test_2013-21_modified_22Mar22.prj, but after saving the shared parameters do not have default values displayed in the Shared Parameters GUI, and this is likely because all shared parameters with HRU dimension have uniformly 14 HRUs dimension.

jhs507 commented 2 years ago

I am still investigating but one thing that I have noticed that in CRHMmain::DoPrjOpen on line 650 there is an explicit exception written for the two parameters in question.

else if (thisPar->param == "obs_elev" || thisPar->param == "soil_withdrawal")
                                    break;

This is likely why the default values are being displayed. Looking at the Borland Code this has been removed and replaced with something else. I will take a look and see if making that change will resolve this or another solution needs to be implemented.

loganxingfang commented 2 years ago

I am still investigating but one thing that I have noticed that in CRHMmain::DoPrjOpen on line 650 there is an explicit exception written for the two parameters in question.

else if (thisPar->param == "obs_elev" || thisPar->param == "soil_withdrawal")
                                  break;

This is likely why the default values are being displayed. Looking at the Borland Code this has been removed and replaced with something else. I will take a look and see if making that change will resolve this or another solution needs to be implemented.

Thanks Justin.

Yes, as mentioned, Tom put some fix in Borland CRHM, but that is not robust enough to handle all exception. This is why there are still issues in Borland CRHM as described in Point 2 and 3.

jhs507 commented 2 years ago

I think I have a fix for this. I have sent you an installer. Try it out and let me know if it resolves things for you.

loganxingfang commented 2 years ago

Thanks Justin. I just downloaded and evaluate the fix in CRHMcode.

The issue mentioned in point 1 is resovled.

The issue mentioned in point 4 is still present. That is, all parameters in Shared were shown as default values in Parameters GUI after saving the prj file ElbowAtYYC_test_mod22Jul2020_with_WRF4km_obs_with_sub_WBvar_estimate_macros.prj. In other words, the common parameters' values among Groups were not able to pasted in Shared after saving the prj file.

jhs507 commented 2 years ago

I think the problem is with the squeeze parameters routine that converts parameters that are identical into shared parameters. This routine is run after the parameters dialog closes. If the routine is coded correctly then repeatedly calling it should have no effect after the first time it is run. However, if you open the parameters dialog and close it repeatedly while the ElbowAtYYC_test_mod22Jul2020_with_WRF4km_obs_with_sub_WBvar_estimate_macros.prj is loaded each time the parameters continue to change.

I did not write the squeeze parameters routine but it is quite complex but I think it should be rewritten to be easier to understand and to ensure that it doesn't have this effect.

loganxingfang commented 2 years ago

Hi Justin, thanks for looking into this. The squeeze parameters routine was written by Tom for Borland CRHM.

The issue is shown up as there are multiple numbers of HRUs in the Shared, ie. numbers of HRUs are not uniform among the shared parameters. If the numbers of HRUs are same among shared Parameters , then saving prj file does not have this issue, as shown in prj file Fortress Mountain Basin test_2013-21_modified_22Mar22a.prj.

I think rewriting some of code could improve this.

loganxingfang commented 2 years ago

The issue is shown up as there are multiple numbers of HRUs in the Shared, ie. numbers of HRUs are not uniform among the shared parameters. If the numbers of HRUs are same among shared Parameters , then saving prj file does not have this issue, as shown in prj file Fortress Mountain Basin test_2013-21_modified_22Mar22a.prj.

To demonstrate this, I created a dummy prj with five groups that all have 14 HRUs. This prj can be found here ElbowAtYYC_test_mod22Jul2020_with_WRF4km_obs_with_sub_WBvar_estimate_macros_5Grps_14HRUs.zip

  1. Then, I saved the above prj in Borland CRHM and the saved prj is here ElbowAtYYC_test_mod22Jul2020_with_WRF4km_obs_with_sub_WBvar_estimate_macros_5Grps_14HRUs_BorlandCRHMsaved.zip

When opening the saved prj in Borland CRHM Parameters GUI, all the shared parameters' values were correctly shown: screen shot 1

Opening the saved prj in TextPad, the shared parameters' values were same as those shown in Parameters GUI, screen shot 2

  1. I repeated the point 1 in CRHMcode and saved the above prj as ElbowAtYYC_test_mod22Jul2020_with_WRF4km_obs_with_sub_WBvar_estimate_macros_5Grps_14HRUs_CRHMCodesaved.zip

When opening the saved prj in CRHMcode Parameters GUI, all the shared parameters' values were correctly shown: screen shot 3

Opening the saved prj in TextPad, the shared parameters' values were same as those shown in Parameters GUI, screen shot 4