srlabUsask / crhmcode

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

CRHMcode GUI - Module order differs from Borland CRHM in some cases. #236

Closed jhs507 closed 2 years ago

jhs507 commented 2 years ago

In the user assessment from December 14, 2021 it is noted that there are slight differences in module ordering from a project built with CRHMcode GUI and Borland CRHM.

See comment 3 from the December 14, 2021 assessment for further detail.

Module order: in this version and also previous versions, I noticed there is slight difference in module order in the prj files built in CRHM_GUI and Borland CRHM. In this version, in the “Example_prj_contruct_CRHM_GUI.prj”, both tsurface#2 and XG modules should be placed after SnobalCRHM#1 module. See the screen shot below for side by side comparison between prj built in Borland CRHM and CRHM_GUI for the module order:

jhs507 commented 2 years ago

Hi Logan,

Does this module ordering difference cause different results?

Do you have any intuition as to why this might be happening?

loganxingfang commented 2 years ago

I will comment this later this week when I have access to my workstation that has CRHM_GUI compiler.

jhs507 commented 2 years ago

No worries thank you.

loganxingfang commented 2 years ago

Hi Justin, The module order matters to the simulation results. Here is explanation:

In the example prj file constructed by CRHM_GUI, ie. Example_Group_prj_contruct_CRHM_GUI.prj file,

  1. the order between tsurface#2 and XG modules is good; that is, tsurface#2 module is above XG module, because input variable "hru_tsf" in XG module is from the output of tsurface#2 module, the variable "hru_tsf" declaration in XG module as:

    declgetvar("*", "hru_tsf", "(°C)", &hru_tsf);

Thus, in the sequence, "hru_tsf" needs to be estimated first by tsurface#2 module and then is read by XG module.

  1. however, both tsurface#2 and XG modules are above SnobalCRHM#1 module, and this is not good order. In this particular variation of tsurface module, ie. tsurface#2, described as 'use previous daily mean snowpack temperature with thermal conductivity and energy of snowpack (Snobal case) to estimate surface temperature during snowcover period; use Radiative-Conductive-Convective approach to estimate surface temperature(Williams et al, 2015) during snow free periods'. There are four variables that are outputs from SnobalCRHM#1 module, listed as below:

    declgetvar("*", "z_s", "(m)", &z_s);

    declgetvar("*", "rho", "(kg/m^3)", &rho);

    declgetvar("*", "T_s", "( °C)", &T_s);

    declgetvar("*", "G", "(W/m^2)", &G);

Thus, in the sequence, these above variables need to be estimated first by SnobalCRHM#1 module and then is read by tsurface#2 module in order to estimate variable "hru_tsf".

In the current order in the Example_Group_prj_contruct_CRHM_GUI.prj file, values of these four variables from SnobalCRHM#1 module are from one time-step prior to the current time-step and thus the estimated "hru_tsf" would reflect the value from one time-step prior.

  1. while in the example prj file constructed by Borland CRHM, ie. the Example_Group_prj_contruct_BorlandCRHM.prj file, both tsurface#2 and XG modules are below SnobalCRHM#1 module, values of these four variables from SnobalCRHM#1 module are from current time-step are read by tsurface#2 module to the estimate "hru_tsf", which subsequently is read by XG module.

I have the module order difference in the screen shot here, example prj constructed by Borland CRHM is on the left while that constructed by CRHM_GUI is on the right. This is also shown in the CRHM_GUI_13Dec2021 features assessment.pdf file. screen shot10

I am not sure why CRHM_GUI has that order, but in Borland CRHM, the module order is determined by the 'hierarchy' of module. I have a screen shot here showing 'hierarchy' in Summary window for Example_Group_prj_contruct_BorlandCRHM.prj file: screen shot53

In the 'hierarchy' window, the value on the left side of module is the 'order' shown as rank value in power of 2, while the value on the right side of module is "intrinsic" value of in CRHM: reflecting module's relevance in the hydrological cycle; value of 0 means the module is commonly needed by all other modules, and the higher value, the latter in the sequence of process in the hydrological cycle.

Note that in the above screen shot, the message "Module Order is not correct". This is because there are three out of order for modules: 1) calcsun module has value of 18 on the right while Slope_Qsi#1 module has 2, but Slope_Qsi#1 is below calcsun, thus this is deemed as incorrect by Borland CRHM. However, Slope_Qsi#1 is an optional module which is used to adjusted the incoming solar radiation on the slope, and it can be placed above or below calcsun, and the order does not impact the result of adjusted Qsi from Slope_Qsi#1 module because Slope_Qsi#1 module reads input observation variable Qsi directly from observation data file.

2) walmsley_wind module with lower value of 4 is placed calcsun module; it can be placed above or below calcsun, and the order does not impact the result of adjusted wind speed from walmsley_wind module because walmsley_wind module reads input variable hru_u directly from obs module, as long as walmsley_wind module is placed below obs module, it is fine.

3) longVt with value of 22 is placed above netall with value of 14, but the order does not impact simulation from both modules, because both modules read input variables from obs module and longVt read additional input observation variable Qsi directly from observation data file. longVt and netall modules do not link directly with each other, thus either longVt above or below netall is fine.

Thus, despite the message "Module Order is not correct" in 'hierarchy' window, the key modules' order is enforced by Borland CRHM and is correct.

I had a similar description of this 'hierarchy' window for "Bad Lake 1974-1975.prj" file in the emails on October 25, 2021.

jhs507 commented 2 years ago

If the modules are always in ascending order based on the intrinsic number on the right is that always a valid configuration?

ie if that is always followed is it possible to make an invalid configuration?

Do variations have different 'intrinsic' numbers than base modules?

loganxingfang commented 2 years ago

If the modules are always in ascending order based on the intrinsic number on the right is that always a valid configuration?

ie if that is always followed is it possible to make an invalid configuration?

Not always, I just demonstrated three instances above. These are modules that are optional, or read inputs directly from module with value of 0, or do not link to each other.

This usually happens to module in the lower order and is fine, as I explained above.

jhs507 commented 2 years ago

In the instances above you showed that it is sometimes okay to have a module with a higher intrinsic value above one with a lower one.

But if all the modules are arranged only on the basis of this intrinsic number is it possible for the ordering to be invalid?

loganxingfang commented 2 years ago

Do variations have different 'intrinsic' numbers than base modules?

This can be the case. Take tsurface module for example.

In the the example prj file, when either of following variation is used, it can be placed above or below SnobalCRHM#1 because that order does not impact simulation and there is no direct link between tsurface and SnobalCRHM#.

tsurface (ie. original variation): 'use hru_t for surface temperature during snowcover period; use Radiative-Conductive-Convective approach to estimate surface temperature(Williams et al, 2015) during snow free periods.'

or tsurface#1 (ie. variation 1)'use hru_t for surface temperature during snowcover period; use parameter n_factor to estimate surface temperature from the air temperature during snow free periods (Woo, 2012, p. 57).'

The model prj constructed using either above variation will run, but it is deemed as not good. Since there is snowmelt model (ie. SnobalCRHM#1), tsurface modules needs to link to snowmelt model for estimating hru_tsf variable. Thus, tsurface#2 is better optional for the hydrological model and needs to be placed below SnobalCRHM#1.

loganxingfang commented 2 years ago

In the instances above you showed that it is sometimes okay to have a module with a higher intrinsic value above one with a lower one.

But if all the modules are arranged only on the basis of this intrinsic number is it possible for the ordering to be invalid?

I can barely keep up with your question and issues.

I already demonstrated and explained this: Note that in the above screen shot, the message "Module Order is not correct". This is because there are three out of order for modules:

calcsun module has value of 18 on the right while Slope_Qsi#1 module has 2, but Slope_Qsi#1 is below calcsun, thus this is deemed as incorrect by Borland CRHM. However, Slope_Qsi#1 is an optional module which is used to adjusted the incoming solar radiation on the slope, and it can be placed above or below calcsun, and the order does not impact the result of adjusted Qsi from Slope_Qsi#1 module because Slope_Qsi#1 module reads input observation variable Qsi directly from observation data file.

walmsley_wind module with lower value of 4 is placed calcsun module; it can be placed above or below calcsun, and the order does not impact the result of adjusted wind speed from walmsley_wind module because walmsley_wind module reads input variable hru_u directly from obs module, as long as walmsley_wind module is placed below obs module, it is fine.

longVt with value of 22 is placed above netall with value of 14, but the order does not impact simulation from both modules, because both modules read input variables from obs module and longVt read additional input observation variable Qsi directly from observation data file. longVt and netall modules do not link directly with each other, thus either longVt above or below netall is fine.

Thus, despite the message "Module Order is not correct" in 'hierarchy' window, the key modules' order is enforced by Borland CRHM and is correct.

jhs507 commented 2 years ago

Do you have time for a zoom call sometime today?

I am clearly not understanding this and maybe a call would help.

loganxingfang commented 2 years ago

After my lunch, I can do a call.

jhs507 commented 2 years ago

Let me know what time works best for you.

loganxingfang commented 2 years ago

I should be good for the call now.

jhs507 commented 2 years ago

I have found the issue here.

In the construct dialog as modules are selected a internal representation of this selection is maintained in a field called SelectedModules. This field is a vector of pairs of a string and a reference to a ClassModlule object. When variations are selected the correct variation number is set in the ClassModule object but a suffix is not appended to the string portion. When the check or build buttons are pressed this list is then reordered to be the correct ordering based on modules requirements. In this process in the CheckModules() function it is assumed that module variation is stored in the string portion of the SelectedModules list of pairs of strings with module objects. Therefore for this reordering it is finding that all the modules are actually using the default variation.

In order to fix I will have the CheckModules() algorithm look in the ClassModule object for the selected variation instead of attempting to derive it from the string portion.

This will also require that when a project is loaded and then the construct dialog loaded that the SelectedModules list also follow this convention. Currently it has the variations stored in the string portion.

jhs507 commented 2 years ago

Closed with https://github.com/srlabUsask/crhmcode/commit/5dffd990de42f784441eab454fc3261bd025b95c