Var_loop_lay_table not being initialized for certain configurations

[lawfordp2017]

I'm trying to get the waterquality modules functioning but having some difficulties. In ClassModule.cpp method initbase() line 72 Var_loop_lay_table gets initialized if Var_NDEFN_cnt > 0. I'm having some problems with this:

first, I can't find this code in the Borland version from 2019. I'm wondering where the code came from. second, the decl and init tasks are not occuring in the proper order so the Var_NDEFN_cnt is not set properly, the Var_loop_lay_table never gets initialized, and later on during initialization at ClassModule.cpp:427 I get a segfault.

I can provide a project file but it may require using the waterquality_updates branch to run, so maybe first we could focus on why Var_loop_lay_table exists in the first place, when it got added in, or whether it is even necessary.

[jhs507]

Thanks, for bringing this to my attention Peter.

I am not familiar with this bit of code off the top of my head I will look into it.

@loganxingfang Do you know where this bit of code originated from?

[jhs507]

The code dealing with Var_loop_lay_table is present as far back as the code being placed into this git repository.

I find no equivalent in the up to date borland source code.

[jhs507]

Another note is that both the Var_loop_lay_table and Par_loop_lay_table variables are only used in ClassModule.cpp they are not referenced elsewhere.

Both only are even used when the declare variable or declare parameter call's dimen parameter is set to TDim::NDEF. The dimen parameter is typically used to indicate the number of dimensions the variable or parameter has. I do not know what TDim::NDEF is intended to represent. It appears to be used in most of the water quality modules so I suspect it was added to accommodate them.

@DiogoCostaPT do you know what these variables are intended for?

[lawfordp2017]

They aren't present in Diogo's WQ version either. I think they existed in the very old borland crhm used by Mani but have no use currently. All code referencing those variables can likely be safely deleted.

[loganxingfang]

For the code involving in Var_loop_lay_table, I am not sure this is from Borland CRHM source. This could be new crhmcode in gitHub.

I searched the ClassModule.cpp file in Borland source code, in that file, on Line 43, void ClassModule::initbase(void) This is probably relevant to ClassModule.cpp file on Line 58 in new crhmcode, where Var_loop_lay_table is used.

For ClassModule.cpp file in Borland source code, there is code using NDEFN on Line 319,

      if((dimen == CRHM::NLAY  || dimen == CRHM::NDEFN) && newVar->lay != dim){
        long JJ = newVar->lay;

this is in the INIT blocks that is called by ClassModule::initbase(void).

Also, there are other lines using NDEFN, on Line 834,

  if((dimen == CRHM::NLAY && layvalue == NULL) || (dimen == CRHM::NDEF && layvalue == NULL) || (dimen == CRHM::NDEFN && layvalue == NULL)) {
    LogError(CRHMException("Layer Array not defined for " + Name + " " + param, WARNING));
    return;
  }

this is in the DECL blocks.

I think NDEFN is to do with dimension for the two-dimensional array.

Also, on Lines 878, 974, 1004, 1011, 1017 in ClassModule.cpp file in Borland source code. There is also code dealing with dimension dimen == CRHM::NDEF in ClassModule.cpp file in Borland source code.

Maybe there is some relevance to what is implemented in new crhmcode.

[lawfordp2017]

NDEFN doesn't get used much except in waterquality modules where it is used to specify the various species of Nitrogen and Phosphorus, which is why I'm coming across this issue now.

[loganxingfang]

I have not really dealt with NDEFN too much before, so I don't know much about it other than what I found above. I think NDEFN can be used in Macro code too to declare dimension for 2-D variable and 2-D parameter.

For NDEF, I think this can be way to declaring dimension for 1-D array. Though I have not seen very much its usage.

In Borland source code file ClassCRHM.cpp,

on Line 876, for parameter dimension, there is code for NDEFN and NDEF:

ClassPar::ClassPar(string module, string param, CRHM::TDim dimen, string valstr, float minVal, float maxVal, string help, string units, CRHM::TVar varType, int defdim, int Grpdim) : module(module), basemodule(""), param(param), varType(varType), dimen(dimen), valstr(valstr), minVal(minVal), maxVal(maxVal), Inhibit_share(0), help(help), units(units), values(NULL), ivalues(NULL), layvalues(NULL), ilayvalues(NULL), Strings(NULL), layvaluesBkup(NULL), ilayvaluesBkup(NULL), Identical(NULL), StringsBkup(NULL), lay(1) {

if(Grpdim == 0 && dimen >= CRHM::NHRU)
  Grpdim = Global::nhru;

if(dimen == CRHM::NLAY) {
  lay = Global::nlay;
  dim = Grpdim;
}
else if(dimen == CRHM::NDEF) {
  lay = defdim;
  dim = 1; //
}
else if(dimen == CRHM::NDEFNZ) {
  lay = defdim;
  dim = 1; // array 1 * n;
}
else if(dimen == CRHM::NDEFN) {
  lay = defdim;
  dim = Grpdim; // Global::nhru;
}
else if(dimen < CRHM::NHRU){
  dim = getdim(dimen); // handle cases of 'ONE, TWO, ...
  lay = 1;
}
else{
  dim = Grpdim;
  lay = 1;
}

try {
  if(varType == CRHM::Float) {
    layvalues = new float *[lay];
    for(int ii = 0; ii < lay; ii++)
      layvalues[ii] = new float[dim];
    values = layvalues[0];
  }
  else if(varType == CRHM::Int) {
    ilayvalues = new long *[lay];
    for(int ii = 0; ii < lay; ii++)
      ilayvalues[ii] = new long[dim];
    ivalues = ilayvalues[0];
  }
}
catch (std::bad_alloc) {
  CRHMException Except("Could not allocate in ClassPar." , TERMINATE);
  LogError(Except);
  throw CRHMException(Except);
}

}

on Line 1241, for variable dimension, there is code for NDEFN and NDEF:

ClassVar::ClassVar(string module, string name, CRHM::TDim dimen, string help, string units, CRHM::TVar varType, bool PointPlot, int Grpdim, int defdim) : module(module), name(name), DLLName(""), root(""), varType(varType), lay(0), nfreq(false), optional(false), StatVar(false), InGroup(0), visibility(CRHM::USUAL), FunKind(CRHM::FOBS), help(help), units(units), layvalues(NULL), ilayvalues(NULL), dim(0), dimMax(0), values(NULL), ivalues(NULL), offset(0), cnt(0), FileData(NULL), HRU_OBS_indexed(0), UserFunct(NULL), FunctVar(NULL), CustomFunct(NULL), No_ReadVar(0), PointPlot(PointPlot), TchrtOpt(0), dimen(dimen) {

if(Grpdim == 0) Grpdim = Global::nhru;

if(dimen == CRHM::NLAY) lay = Global::nlay; else if(dimen == CRHM::NFREQ){ lay = Global::Freq; nfreq = true; } else if(dimen == CRHM::NDEF){ lay = defdim; dim = 1; } else if(dimen == CRHM::NDEFN) { lay = defdim; dim = Grpdim; } else if(dimen == CRHM::NREB) lay = Grpdim; // memory allocated by variables found else lay = 0;

if(dimen == CRHM::NOBS) dim = Global::nobs; else if(dimen != CRHM::NDEF) dim = Grpdim;

try { if(varType == CRHM::Float) { if(lay > 0) { layvalues = new float *[lay]; if(!values) values = new float[dim]; if(dimen != CRHM::NREB){ // NREB does not own lay memory only HRU memory for(int ii = 0; ii < lay; ii++) layvalues[ii] = new float[dim]; values = layvalues[0]; // sets to first layer

      for(int jj = 0; jj < lay; ++jj)
        for(int kk = 0; kk < dim; ++kk)
          layvalues[jj][kk] = 0.0;
    }
  }

  if(lay == 0 || dimen == CRHM::NREB) {
    values = new float[dim];
    for(int kk = 0; kk < dim; ++kk)
      values[kk] = 0.0;
  }
}
else if(varType == CRHM::Int) {
  if(lay > 0) {
    ilayvalues = new long *[lay];
    if(dimen != CRHM::NREB){ // NREB does not own lay memory only HRU memory
      for(int ii = 0; ii < lay; ii++)
        ilayvalues[ii] = new long[dim];
      ivalues = ilayvalues[0];

      for(int jj = 0; jj < lay; ++jj)
        for(int kk = 0; kk < dim; ++kk)
          ilayvalues[jj][kk] = 0.0;
    }
  }

  if(lay == 0 || dimen == CRHM::NREB) {
    ivalues = new long[dim];
    for(int kk = 0; kk < dim; ++kk)
      ivalues[kk] = 0;
  }
}

} catch (std::bad_alloc) { CRHMException Except("Could not allocate in ClassVar." , TERMINATE); LogError(Except); throw CRHMException(Except); } }

[DiogoCostaPT]

Hi all,

Yes, if I remember well this is used to set up the number of chemical species. Because CRHM did not have a proper way for expanding the simulations to multiple species (multiple state-variables for each compartment), Tom though it could go in lay as that would offer already this multi-dimensional expansion. Not sure it’s the best way of doing it, but I understand why he did it. Creating such multi-dimensional arrays (for each compartment) to account for WQ would be a change that was too structural and risky. So, he tried to work with what was already in there.

On 26 May 2022, at 21:31, loganxingfang @.***> wrote:

CAUTION: External to USask. Verify sender and use caution with links and attachments. Forward suspicious emails to @.***

I have not really dealt with NDEFN too much before, so I don't know much about it other than what I found above. I think NDEFN can be used in Macro code too to declare dimension for 2-D variable and 2-D parameter.

For NDEF, I think this can be way to declaring dimension for 1-D array. Though I have not seen very much its usage.

In Borland source code file ClassCRHM.cpp,

on Line 876, for parameter dimension, there is code for NDEFN and NDEF:

ClassPar::ClassPar(string module, string param, CRHM::TDim dimen, string valstr, float minVal, float maxVal, string help, string units, CRHM::TVar varType, int defdim, int Grpdim) : module(module), basemodule(""), param(param), varType(varType), dimen(dimen), valstr(valstr), minVal(minVal), maxVal(maxVal), Inhibit_share(0), help(help), units(units), values(NULL), ivalues(NULL), layvalues(NULL), ilayvalues(NULL), Strings(NULL), layvaluesBkup(NULL), ilayvaluesBkup(NULL), Identical(NULL), StringsBkup(NULL), lay(1) {

if(Grpdim == 0 && dimen >= CRHM::NHRU) Grpdim = Global::nhru;

if(dimen == CRHM::NLAY) { lay = Global::nlay; dim = Grpdim; } else if(dimen == CRHM::NDEF) { lay = defdim; dim = 1; // } else if(dimen == CRHM::NDEFNZ) { lay = defdim; dim = 1; // array 1 * n; } else if(dimen == CRHM::NDEFN) { lay = defdim; dim = Grpdim; // Global::nhru; } else if(dimen < CRHM::NHRU){ dim = getdim(dimen); // handle cases of 'ONE, TWO, ... lay = 1; } else{ dim = Grpdim; lay = 1; }

try { if(varType == CRHM::Float) { layvalues = new float [lay]; for(int ii = 0; ii < lay; ii++) layvalues[ii] = new float[dim]; values = layvalues[0]; } else if(varType == CRHM::Int) { ilayvalues = new long [lay]; for(int ii = 0; ii < lay; ii++) ilayvalues[ii] = new long[dim]; ivalues = ilayvalues[0]; } } catch (std::bad_alloc) { CRHMException Except("Could not allocate in ClassPar." , TERMINATE); LogError(Except); throw CRHMException(Except); }

}

on Line 1241, for variable dimension, there is code for NDEFN and NDEF:

ClassVar::ClassVar(string module, string name, CRHM::TDim dimen, string help, string units, CRHM::TVar varType, bool PointPlot, int Grpdim, int defdim) : module(module), name(name), DLLName(""), root(""), varType(varType), lay(0), nfreq(false), optional(false), StatVar(false), InGroup(0), visibility(CRHM::USUAL), FunKind(CRHM::FOBS), help(help), units(units), layvalues(NULL), ilayvalues(NULL), dim(0), dimMax(0), values(NULL), ivalues(NULL), offset(0), cnt(0), FileData(NULL), HRU_OBS_indexed(0), UserFunct(NULL), FunctVar(NULL), CustomFunct(NULL), No_ReadVar(0), PointPlot(PointPlot), TchrtOpt(0), dimen(dimen) {

if(Grpdim == 0) Grpdim = Global::nhru;

if(dimen == CRHM::NLAY) lay = Global::nlay; else if(dimen == CRHM::NFREQ){ lay = Global::Freq; nfreq = true; } else if(dimen == CRHM::NDEF){ lay = defdim; dim = 1; } else if(dimen == CRHM::NDEFN) { lay = defdim; dim = Grpdim; } else if(dimen == CRHM::NREB) lay = Grpdim; // memory allocated by variables found else lay = 0;

if(dimen == CRHM::NOBS) dim = Global::nobs; else if(dimen != CRHM::NDEF) dim = Grpdim;

try { if(varType == CRHM::Float) { if(lay > 0) { layvalues = new float *[lay]; if(!values) values = new float[dim]; if(dimen != CRHM::NREB){ // NREB does not own lay memory only HRU memory for(int ii = 0; ii < lay; ii++) layvalues[ii] = new float[dim]; values = layvalues[0]; // sets to first layer

  for(int jj = 0; jj < lay; ++jj)
    for(int kk = 0; kk < dim; ++kk)
      layvalues[jj][kk] = 0.0;
}

}

if(lay == 0 || dimen == CRHM::NREB) { values = new float[dim]; for(int kk = 0; kk < dim; ++kk) values[kk] = 0.0; } } else if(varType == CRHM::Int) { if(lay > 0) { ilayvalues = new long *[lay]; if(dimen != CRHM::NREB){ // NREB does not own lay memory only HRU memory for(int ii = 0; ii < lay; ii++) ilayvalues[ii] = new long[dim]; ivalues = ilayvalues[0];

  for(int jj = 0; jj < lay; ++jj)
    for(int kk = 0; kk < dim; ++kk)
      ilayvalues[jj][kk] = 0.0;
}

}

if(lay == 0 || dimen == CRHM::NREB) { ivalues = new long[dim]; for(int kk = 0; kk < dim; ++kk) ivalues[kk] = 0; } }

} catch (std::bad_alloc) { CRHMException Except("Could not allocate in ClassVar." , TERMINATE); LogError(Except); throw CRHMException(Except); } }

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

[jhs507]

Thank you Diogo,

I will have to take a closer look at this later but I suspect we will want to refactor how these are tracked in CRHM to be easier to understand.

As far as the original issue that Peter brought up with Var_loop_lay_table it does seem to me that we could move forward by removing it and see if it has any ill effects. If some multi dimensional array is needed to track various species for water quality purposes I propose that we introduce a class that derives from ClassModule for this purpose.

[DiogoCostaPT]

That’s fine. I think that some serious thinking will have to be put on the best way to re-integrate the WQ modules in the new CHRM version. Has anyone started on that?

I have been working on a completely different version of the water quality modules, which we are coupling to SUMMA and CHRM (old Linux version). It might be worth talking when anyone is planning to integrate those older WQ modules.

Sent from my iPhone

On May 31, 2022, at 8:24 PM, jhs507 @.***> wrote:

 CAUTION: External to USask. Verify sender and use caution with links and attachments. Forward suspicious emails to @.***

Thank you Diogo,

I will have to take a closer look at this later but I suspect we will want to refactor how these are tracked in CRHM to be easier to understand.

As far as the original issue that Peter brought up with Var_loop_lay_table it does seem to me that we could move forward by removing it and see if it has any ill effects. If some multi dimensional array is needed to track various species for water quality purposes I propose that we introduce a class that derives from ClassModule for this purpose.

— Reply to this email directly, view it on GitHubhttps://github.com/srlabUsask/crhmcode/issues/308#issuecomment-1142555463, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AFC5FPYJWL2GTGSYAFE2RBTVMZRQNANCNFSM5XB6SM4A. You are receiving this because you were mentioned.Message ID: @.***>

[jhs507]

I am still busy with getting the GUI re-implemented so that we can release CRHMcode.

I think that Peter may be working with the water quality modules.

[lawfordp2017]

Diogo, I've made some modifications to the erosion code in crhm-wq as well as adding a couple new modules such as WQ_REWroute. Still in testing and I need to check with you whether the code is correct.

[jhs507]

@lawfordp2017 Can you give me an update on this issue?

[DiogoCostaPT]

Hi @lawfordp2017

Good to hear from you. I haven't worked with this for a few years. Can you please give me more precise instructions on the clarifications you need and how I can help you with this issue?

[lawfordp2017]

As far as I'm concerned this issue has been resolved. Taufique has been able to run the water quality modules on his own basin data.

[DiogoCostaPT]

perfect

From: jhs507 @.> Date: Tuesday, 4 June 2024 at 16:58 To: srlabUsask/crhmcode @.> Cc: Diogo Costa @.>, Mention @.> Subject: Re: [srlabUsask/crhmcode] Var_loop_lay_table not being initialized for certain configurations (Issue #308)

Closed #308https://github.com/srlabUsask/crhmcode/issues/308 as completed.

— Reply to this email directly, view it on GitHubhttps://github.com/srlabUsask/crhmcode/issues/308#event-13038768948, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AFC5FP5NWOEW5PARIX4ZGNTZFXP2DAVCNFSM5XB6SM4KU5DIOJSWCZC7NNSXTWQAEJEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW4OZRGMYDGOBXGY4DSNBY. You are receiving this because you were mentioned.Message ID: @.***>