srlabUsask / crhmcode

GNU General Public License v3.0
7 stars 3 forks source link

Module hierarchy for glacier and Ayers #433

Closed loganxingfang closed 10 months ago

loganxingfang commented 11 months ago

I constructed example prj files in both Borland CRHM and CRHMcode CRHM_GUI.

The prj file constructed in Borland CRHM is here: LGRB_new_prj_built2.zip

The prj file constructed in CRHMcode CRHM_GUI is here: LGRB_new_prj_built1_CRHM_GUI.zip

For both Borland CRHM and CRHMcode CRHM_GUI, in the Build/Construct window, when hitting "Build", there will be messages showing "Module not used! Remove module?" for three modules K_Estimate, SWESlope, and glacier; this appears because neither input nor output variables from these modules are needed by other modules and there are other modules generating the same input or output variable.

The following is module order from LGRB_new_prj_built2.prj:

basin CRHM 02/24/12 global CRHM 12/19/19 obs CRHM 06/28/22 K_Estimate CRHM 04/06/22 Slope_Qsi#1 CRHM 07/14/11 calcsun#1 CRHM 10/01/13 walmsley_wind CRHM 07/30/08 longVt#3 CRHM 04/04/22 netall CRHM 04/04/22 evap CRHM 03/18/22 CanopyClearingGap#3 CRHM 04/05/22 albedo_Richard CRHM 03/19/15 pbsmSnobal#1 CRHM 01/05/17 SnobalCRHM#1 CRHM 11/21/16 SWESlope#1 CRHM 04/05/22 Ayers CRHM 04/05/22 glacier#5 CRHM 04/22/22 Soil CRHM 04/05/22 Netroute CRHM 04/05/22

The following is module order from LGRB_new_prj_built1_CRHM_GUI.prj:

basin CRHM 02/24/12 global CRHM 12/19/19 obs CRHM 04/17/18 K_Estimate CRHM 04/06/22 Slope_Qsi#1 CRHM 07/14/11 walmsley_wind CRHM 07/30/08 calcsun#1 CRHM 10/01/13 longVt#3 CRHM 04/04/22 netall CRHM 04/04/22 evap CRHM 03/18/22 CanopyClearingGap#3 CRHM 04/05/22 albedo_Richard CRHM 03/19/15 pbsmSnobal#1 CRHM 01/05/17 SnobalCRHM#1 CRHM 11/21/16 SWESlope#1 CRHM 04/05/22 Ayers CRHM 04/05/22 glacier#5 CRHM 04/22/22 Soil CRHM 04/05/22 Netroute CRHM 04/05/22

For both modules K_Estimate, SWESlope#1, their module is good shown above.

For module glacier#5, its module order is not correct in regards to module Ayers. The reason for incorrect order shown above is because module glacier always has slightly higher hierarchy value than infiltration module such as Ayers or frozenAyers.

The hierarchy value can be reported in Borland CRHM's Report - Hierarchy;

Here is the hierarchy value for LGRB_new_prj_built2.prj: hierarchy_LGRB_new_prj_built2

Here is the hierarchy value for LGRB_new_prj_built1_CRHM_GUI.prj: hierarchy_LGRB_new_prj_built1_CRHM_GUI

As shown in above screen shot, glacier always has slightly higher hierarchy value than infiltration module such as Ayers, and this determines the module order when constructing modules in in the Build/Construct window.

Anyway, I also tried module frozenAyers and found module glacier is placed below frozenAyers. I think this will also occur to other infiltration module such as crack, PrairieInfiltration.

Generally speaking, the module glacier should be placed above infiltration module Ayers, frozenAyers, crack, Greencrack, GreenAmpt, or PrairieInfiltration. The reason for this is because output variable net_rain from module glacier needs to be read by infiltration module in addition to net_rain from module such as snobalCRHM.

loganxingfang commented 11 months ago

In Borland CRHM source code, there is file "Bld.cpp" that contains the code to determine the module order from their rank. Particularly, line 930 to line 968 could be relevant when dealing with equal rank module or module not needed.

jhs507 commented 11 months ago

In crhmcode this code is in ConstructDlg.cpp starting at line 1538 but I can't find where the initial module rank is set from.

jhs507 commented 11 months ago

I am a bit confused Glacier is above Ayers in both models above. So is this error present in both borland and crhmcode?

loganxingfang commented 11 months ago

I am a bit confused Glacier is above Ayers in both models above. So is this error present in both borland and crhmcode?

In both examples shown above, the order is not correct, that is, glacier#5 is placed below Ayers:

Ayers CRHM 04/05/22 glacier#5 CRHM 04/22/22

This wrong order will not allow glacier#5 output variable net_rain to be read by module Ayers.

The correct order should be module glacier#5 being placed above Ayers:

glacier#5 CRHM 04/22/22 Ayers CRHM 04/05/22

jhs507 commented 11 months ago

Okay thank you for the clarification. So this error is in both borland and crhmcode.

loganxingfang commented 11 months ago

That is right. I think it could be an exception or case when building prj using GUI that is not handled in "Bld.cpp" for Borland CRHM and "ConstructDlg.cpp" for crhmcode CRHM_GUI. Something to do with rank in both .cpp files, and correction to this would move glacier module down immediately below ebsm or snobalCRHM module but move glacier module up above Ayers, frozenAyers, crack, Greencrack, GreenAmpt, or PrairieInfiltration module.

jhs507 commented 11 months ago

So from what I can tell a module's rank is determined by looking at the other modules in the model that it uses. This gives each module a rank number. Then the modules are sorted by rank and ties are broken by the "peer rank" value. Glacier and Ayers have the same peer rank so they don't end up being moved. By changing their peer rank this could fix this situation but I don't understand why if net_rain is produced by glacier and required by Ayers the regular process doesn't rank them properly.

loganxingfang commented 11 months ago

Module glacier is not absolutely needed, that is why there is meesage "Module not used! Remove module?". This is because variable net_rain is generated by module CanopyClearingGap#3 first and then is modified by module snobalCRHM. With or without module glacier, the model would be built successfully.

The above is handed in "Bld.cpp" as following,

bool Removed = false;
for(unsigned long ii = 0; ii <  Entries.size()-1; ++ii){
  if(!Entries[ii].used && Entries[ii].Name != "basin"){  // never remove basin
    int result = IDYES;
    result = Application->MessageBox(Entries[ii].Name.c_str(), "Module not used!  Remove module?", MB_YESNO);
    if(result != IDYES)
      continue;
    int indx = IndexOf(ListBox2, Entries[ii].Name);
    ListBox2->Items->Delete(indx);
    Removed = true;
  }
}

For the "peer rank", in "Bld.cpp", it is sorted in alphabetic order, which explains Ayers or frozenAyers is above glacier,

// move entries with equal rank together in alphabetic order or peer rank for(long ii = 0; ii < Entries.size()-1; ++ii) { ClassModule E2 = (ClassModule) Entries[ii].module; if(E2->PeerRank < 0) Entries[ii].used = true; for(long jj = ii+1; jj < Entries.size(); ++jj) { if(Entries[jj].rank == Entries[ii].rank) { ClassModule E1 = (ClassModule) Entries[jj].module;

        if(labs(E1->PeerRank) < labs(E2->PeerRank)){
          MoveDown(Entries, ii, jj);
          Loop = true;
        }
      } // if
    } // for
  } // for
loganxingfang commented 11 months ago

If this issue in "peer rank" cannot be handled easily in "Bld.cpp" for Borland CRHM and "ConstructDlg.cpp" for crhmcode CRHM_GUI, I have a possible easier solution. For those infiltration modules, they can have a variation, e.g. variation1 that declares a variable to get from module glacier, likely declgetvar(); this invoke module glacier and will move infiltration module down in module order. I will get back on this tomorrow.

jhs507 commented 11 months ago

Peer rank has a default value of 1000 set by the constructor for the module. The comments of the code say that things are sorted alphabetically but as far as I can tell this isn't actually guaranteed to be the case.

Altering the value of peer rank for these certain modules would allow you to fix these edge cases but choosing correct values for them that apply in all edge cases would be a difficult task. This could be done as a stop gap measure.

The module ordering code is some of the hardest to understand in the system and I think a rewrite of it would be beneficial in the long run but would take considerable resources and testing.

loganxingfang commented 11 months ago

I have just been looking into this in NewModules.h file for infiltration modules. I have added code to handle their rank with respect to glacier module. I test it and it solves this issue. I will have more details about this shortly.

loganxingfang commented 11 months ago

In NewModules.h file, there is code to explicitly set the Peer rank, and I took a look at that for infiltration modules Ayers, frozenAyers, frozen, crack, Greencrack, GreenAmpt, and PrairieInfiltration, and the Peer rank is left not set, ie. probably with default value of 1000, see below for these modules:

ClassAyers(string Name, String Version = "undefined", CRHM::LMODULE Lvl = CRHM::PROTO) : ClassModule(Name, Version, Lvl) {};

Classfrozen(string Name, String Version = "undefined", CRHM::LMODULE Lvl = CRHM::PROTO) : ClassModule(Name, Version, Lvl), infiltype(NULL) {};

ClassfrozenAyers(string Name, String Version = "undefined", CRHM::LMODULE Lvl = CRHM::PROTO) : ClassModule(Name, Version, Lvl), infiltype(NULL) {};

Classcrack(string Name, String Version = "undefined", CRHM::LMODULE Lvl = CRHM::PROTO) : ClassModule(Name, Version, Lvl), Xinfil(NULL), timer(NULL) {};

ClassGreencrack(string Name, String Version = "undefined", CRHM::LMODULE Lvl = CRHM::PROTO) : ClassModule(Name, Version, Lvl), Xinfil(NULL), timer(NULL) {};

ClassPrairieInfil(string Name, String Version = "undefined", CRHM::LMODULE Lvl = CRHM::PROTO) : ClassModule(Name, Version, Lvl), Xinfil(NULL), timer(NULL) {};

ClassGreenAmpt(string Name, String Version = "undefined", CRHM::LMODULE Lvl = CRHM::PROTO) : ClassModule(Name, Version, Lvl) {};

Then, I added code with Peer rank value 1001 and also a variable "net_rain_org", see below for these modules with this addition:

ClassAyers(string Name, String Version = "undefined", CRHM::LMODULE Lvl = CRHM::PROTO) : ClassModule(Name, Version, Lvl, 1001, "net_rain_org") {}; // setting PeerRank, change on 04Oct2023

ClassfrozenAyers(string Name, String Version = "undefined", CRHM::LMODULE Lvl = CRHM::PROTO) : ClassModule(Name, Version, Lvl, 1001, "net_rain_org"), // setting PeerRank, change on 04Oct2023 infiltype(NULL) {};

Classfrozen(string Name, String Version = "undefined", CRHM::LMODULE Lvl = CRHM::PROTO) : ClassModule(Name, Version, Lvl, 1001, "net_rain_org"), // setting PeerRank, change on 04Oct2023 infiltype(NULL) {};

Classcrack(string Name, String Version = "undefined", CRHM::LMODULE Lvl = CRHM::PROTO) : ClassModule(Name, Version, Lvl, 1001, "net_rain_org"), // setting PeerRank, change on 04Oct2023 Xinfil(NULL), timer(NULL) {};

ClassGreencrack(string Name, String Version = "undefined", CRHM::LMODULE Lvl = CRHM::PROTO) : ClassModule(Name, Version, Lvl, 1001, "net_rain_org"), // setting PeerRank, change on 04Oct2023 Xinfil(NULL), timer(NULL) {};

ClassGreenAmpt(string Name, String Version = "undefined", CRHM::LMODULE Lvl = CRHM::PROTO) : ClassModule(Name, Version, Lvl, 1001, "net_rain_org") {}; // setting PeerRank, change on 04Oct2023

ClassPrairieInfil(string Name, String Version = "undefined", CRHM::LMODULE Lvl = CRHM::PROTO) : ClassModule(Name, Version, Lvl, 1001, "net_rain_org"), // setting PeerRank, change on 04Oct2023 Xinfil(NULL), timer(NULL) {};

With the addition, during model project construct in the GUI, users select any of these infiltration modules being selected along with glacier module, the infiltration module will be moved down below glacier, because infiltration module invokes the output variable net_rain_org from glacier module even though this variable net_rain_org is not being declared and used in the infiltration module.

I compiled the new Borland CRHM and constructed seven model project files using one these seven infiltration modules along with glacier module, and the module order is correct now. See below for these:

  1. Ayers with glacier:

    Modules:

    basin CRHM 02/24/12 global CRHM 12/19/19 obs CRHM 06/28/22 K_Estimate CRHM 04/06/22 Slope_Qsi#1 CRHM 07/14/11 calcsun#1 CRHM 10/01/13 walmsley_wind CRHM 07/30/08 longVt#3 CRHM 04/04/22 netall CRHM 04/04/22 evap CRHM 03/18/22 CanopyClearingGap#3 CRHM 04/05/22 albedo_Richard CRHM 03/19/15 pbsmSnobal#1 CRHM 01/05/17 SnobalCRHM#1 CRHM 11/21/16 SWESlope#1 CRHM 04/05/22 glacier#5 CRHM 04/22/22 Ayers CRHM 04/05/22 Soil CRHM 04/05/22 Netroute CRHM 04/05/22

  2. frozenAyers with glacier:

    Modules:

    basin CRHM 02/24/12 global CRHM 12/19/19 obs CRHM 06/28/22 K_Estimate CRHM 04/06/22 Slope_Qsi#1 CRHM 07/14/11 calcsun#1 CRHM 10/01/13 walmsley_wind CRHM 07/30/08 longVt#3 CRHM 04/04/22 netall CRHM 04/04/22 evap CRHM 03/18/22 CanopyClearingGap#3 CRHM 04/05/22 albedo_Richard CRHM 03/19/15 pbsmSnobal#1 CRHM 01/05/17 SnobalCRHM#1 CRHM 11/21/16 SWESlope#1 CRHM 04/05/22 glacier#5 CRHM 04/22/22 frozenAyers CRHM 01/09/23 Soil CRHM 04/05/22 Netroute CRHM 04/05/22

  3. frozen with glacier:

    Modules:

    basin CRHM 02/24/12 global CRHM 12/19/19 obs CRHM 06/28/22 K_Estimate CRHM 04/06/22 Slope_Qsi#1 CRHM 07/14/11 calcsun#1 CRHM 10/01/13 walmsley_wind CRHM 07/30/08 longVt#3 CRHM 04/04/22 netall CRHM 04/04/22 evap CRHM 03/18/22 CanopyClearingGap#3 CRHM 04/05/22 albedo_Richard CRHM 03/19/15 pbsmSnobal#1 CRHM 01/05/17 SnobalCRHM#1 CRHM 11/21/16 SWESlope#1 CRHM 04/05/22 glacier#5 CRHM 04/22/22 frozen CRHM 01/09/23 Soil CRHM 04/05/22 Netroute CRHM 04/05/22

  4. crack with glacier

    Modules:

    basin CRHM 02/24/12 global CRHM 12/19/19 obs CRHM 06/28/22 K_Estimate CRHM 04/06/22 Slope_Qsi#1 CRHM 07/14/11 calcsun#1 CRHM 10/01/13 walmsley_wind CRHM 07/30/08 longVt#3 CRHM 04/04/22 netall CRHM 04/04/22 evap CRHM 03/18/22 CanopyClearingGap#3 CRHM 04/05/22 albedo_Richard CRHM 03/19/15 pbsmSnobal#1 CRHM 01/05/17 SnobalCRHM#1 CRHM 11/21/16 SWESlope#1 CRHM 04/05/22 glacier#5 CRHM 04/22/22 crack CRHM 04/04/22 Soil CRHM 04/05/22 Netroute CRHM 04/05/22

  5. Greencrack with glacier:

    Modules:

    basin CRHM 02/24/12 global CRHM 12/19/19 obs CRHM 06/28/22 K_Estimate CRHM 04/06/22 Slope_Qsi#1 CRHM 07/14/11 calcsun#1 CRHM 10/01/13 walmsley_wind CRHM 07/30/08 longVt#3 CRHM 04/04/22 netall CRHM 04/04/22 evap CRHM 03/18/22 CanopyClearingGap#3 CRHM 04/05/22 albedo_Richard CRHM 03/19/15 pbsmSnobal#1 CRHM 01/05/17 SnobalCRHM#1 CRHM 11/21/16 SWESlope#1 CRHM 04/05/22 glacier#5 CRHM 04/22/22 Greencrack CRHM 04/04/22 Soil CRHM 04/05/22 Netroute CRHM 04/05/22

  6. GreenAmpt with glacier:

    Modules:

    basin CRHM 02/24/12 global CRHM 12/19/19 obs CRHM 06/28/22 K_Estimate CRHM 04/06/22 Slope_Qsi#1 CRHM 07/14/11 calcsun#1 CRHM 10/01/13 walmsley_wind CRHM 07/30/08 longVt#3 CRHM 04/04/22 netall CRHM 04/04/22 evap CRHM 03/18/22 CanopyClearingGap#3 CRHM 04/05/22 albedo_Richard CRHM 03/19/15 pbsmSnobal#1 CRHM 01/05/17 SnobalCRHM#1 CRHM 11/21/16 SWESlope#1 CRHM 04/05/22 glacier#5 CRHM 04/22/22 GreenAmpt CRHM 04/04/22 Soil CRHM 04/05/22 Netroute CRHM 04/05/22

  7. PrairieInfiltration with glacier:

    Modules:

    basin CRHM 02/24/12 global CRHM 12/19/19 obs CRHM 06/28/22 K_Estimate CRHM 04/06/22 Slope_Qsi#1 CRHM 07/14/11 calcsun#1 CRHM 10/01/13 walmsley_wind CRHM 07/30/08 longVt#3 CRHM 04/04/22 netall CRHM 04/04/22 evap CRHM 03/18/22 CanopyClearingGap#3 CRHM 04/05/22 albedo_Richard CRHM 03/19/15 pbsmSnobal#1 CRHM 01/05/17 SnobalCRHM#1 CRHM 11/21/16 SWESlope#1 CRHM 04/05/22 glacier#5 CRHM 04/22/22 PrairieInfiltration CRHM 04/05/22 Soil CRHM 04/05/22 Netroute CRHM 04/05/22

In addition, I also constructed seven model projects without glacier module to see if addition of ClassModule(Name, Version, Lvl, 1001, "net_rain_org") would affect module order for project without glacier module. I can confirm the module order is correct, see below for these without glacier module:

  1. Ayers without glacier:

    Modules:

    basin CRHM 02/24/12 global CRHM 12/19/19 obs CRHM 06/28/22 K_Estimate CRHM 04/06/22 Slope_Qsi#1 CRHM 07/14/11 calcsun#1 CRHM 10/01/13 walmsley_wind CRHM 07/30/08 longVt#3 CRHM 04/04/22 netall CRHM 04/04/22 evap CRHM 03/18/22 CanopyClearingGap#3 CRHM 04/05/22 albedo_Richard CRHM 03/19/15 pbsmSnobal#1 CRHM 01/05/17 SnobalCRHM#1 CRHM 11/21/16 SWESlope#1 CRHM 04/05/22 Ayers CRHM 04/05/22 Soil CRHM 04/05/22 Netroute CRHM 04/05/22

  2. frozenAyers without glacier:

    Modules:

    basin CRHM 02/24/12 global CRHM 12/19/19 obs CRHM 06/28/22 K_Estimate CRHM 04/06/22 Slope_Qsi#1 CRHM 07/14/11 calcsun#1 CRHM 10/01/13 walmsley_wind CRHM 07/30/08 longVt#3 CRHM 04/04/22 netall CRHM 04/04/22 evap CRHM 03/18/22 CanopyClearingGap#3 CRHM 04/05/22 albedo_Richard CRHM 03/19/15 pbsmSnobal#1 CRHM 01/05/17 SnobalCRHM#1 CRHM 11/21/16 SWESlope#1 CRHM 04/05/22 frozenAyers CRHM 01/09/23 Soil CRHM 04/05/22 Netroute CRHM 04/05/22

  3. frozen without glacier:

    Modules:

    basin CRHM 02/24/12 global CRHM 12/19/19 obs CRHM 06/28/22 K_Estimate CRHM 04/06/22 Slope_Qsi#1 CRHM 07/14/11 calcsun#1 CRHM 10/01/13 walmsley_wind CRHM 07/30/08 longVt#3 CRHM 04/04/22 netall CRHM 04/04/22 evap CRHM 03/18/22 CanopyClearingGap#3 CRHM 04/05/22 albedo_Richard CRHM 03/19/15 pbsmSnobal#1 CRHM 01/05/17 SnobalCRHM#1 CRHM 11/21/16 SWESlope#1 CRHM 04/05/22 frozen CRHM 01/09/23 Soil CRHM 04/05/22 Netroute CRHM 04/05/22

  4. crack without glacier

    Modules:

    basin CRHM 02/24/12 global CRHM 12/19/19 obs CRHM 06/28/22 K_Estimate CRHM 04/06/22 Slope_Qsi#1 CRHM 07/14/11 calcsun#1 CRHM 10/01/13 walmsley_wind CRHM 07/30/08 longVt#3 CRHM 04/04/22 netall CRHM 04/04/22 evap CRHM 03/18/22 CanopyClearingGap#3 CRHM 04/05/22 albedo_Richard CRHM 03/19/15 pbsmSnobal#1 CRHM 01/05/17 SnobalCRHM#1 CRHM 11/21/16 SWESlope#1 CRHM 04/05/22 crack CRHM 04/04/22 Soil CRHM 04/05/22 Netroute CRHM 04/05/22

  5. Greencrack without glacier:

    Modules:

    basin CRHM 02/24/12 global CRHM 12/19/19 obs CRHM 06/28/22 K_Estimate CRHM 04/06/22 Slope_Qsi#1 CRHM 07/14/11 calcsun#1 CRHM 10/01/13 walmsley_wind CRHM 07/30/08 longVt#3 CRHM 04/04/22 netall CRHM 04/04/22 evap CRHM 03/18/22 CanopyClearingGap#3 CRHM 04/05/22 albedo_Richard CRHM 03/19/15 pbsmSnobal#1 CRHM 01/05/17 SnobalCRHM#1 CRHM 11/21/16 SWESlope#1 CRHM 04/05/22 Greencrack CRHM 04/04/22 Soil CRHM 04/05/22 Netroute CRHM 04/05/22

  6. GreenAmpt without glacier:

    Modules:

    basin CRHM 02/24/12 global CRHM 12/19/19 obs CRHM 06/28/22 K_Estimate CRHM 04/06/22 Slope_Qsi#1 CRHM 07/14/11 calcsun#1 CRHM 10/01/13 walmsley_wind CRHM 07/30/08 longVt#3 CRHM 04/04/22 netall CRHM 04/04/22 evap CRHM 03/18/22 CanopyClearingGap#3 CRHM 04/05/22 albedo_Richard CRHM 03/19/15 pbsmSnobal#1 CRHM 01/05/17 SnobalCRHM#1 CRHM 11/21/16 SWESlope#1 CRHM 04/05/22 GreenAmpt CRHM 04/04/22 Soil CRHM 04/05/22 Netroute CRHM 04/05/22

  7. PrairieInfiltration without glacier:

    Modules:

    basin CRHM 02/24/12 global CRHM 12/19/19 obs CRHM 06/28/22 K_Estimate CRHM 04/06/22 Slope_Qsi#1 CRHM 07/14/11 calcsun#1 CRHM 10/01/13 walmsley_wind CRHM 07/30/08 longVt#3 CRHM 04/04/22 netall CRHM 04/04/22 evap CRHM 03/18/22 CanopyClearingGap#3 CRHM 04/05/22 albedo_Richard CRHM 03/19/15 pbsmSnobal#1 CRHM 01/05/17 SnobalCRHM#1 CRHM 11/21/16 SWESlope#1 CRHM 04/05/22 PrairieInfiltration CRHM 04/05/22 Soil CRHM 04/05/22 Netroute CRHM 04/05/22

I will create a new branch and a pull request for crhmcode GUI.

jhs507 commented 11 months ago

Perfect, Thank you logan.