Avoid use of ion variable in the CONSTANT block

[pramodk]

• certain mod files use read/write ion variables
  from USEION statements in CONSTANT {} block

• the generated code from such usage is not desired
  I believe. For example, cdp_spiny.mod from ModelDB
  model id 266799 has

  NEURON {
  	SUFFIX cdp4Nsp
   	USEION ca READ cao, cai, ica WRITE cai
   	RANGE ica_pmp,scale
    ...
  }
  
  ASSIGNED {
     diam      (um)
     ica       (mA/cm2)
     ica_pmp   (mA/cm2)
     parea     (um)     : pump area per unit length
     cai       (mM)
  }
  
  CONSTANT { cao = 2  (mM) }

  In this example, the generated code doesn't set ion variable cao to 2
  but declare a static variable cao with value 2 and that is not used
  anywhere:

   #define ica _p[56]
   #define ica_columnindex 56
   #define parea _p[57]
   #define parea_columnindex 57
   #define cai _p[58]
   ...
   #define _ion_cao    *_ppvar[0]._pval
   #define _ion_cai    *_ppvar[1]._pval
   #define _ion_ica    *_ppvar[2]._pval
   #define _style_ca   *((int*)_ppvar[3]._pvoid)
   #define diam    *_ppvar[4]._pval
   ...
   static double cao = 2;
   ...
   static void nrn_state(NrnThread* _nt, _Memb_list* _ml, int _type) {
  
    ...
    cao = _ion_cao;
        cai = _ion_cai;
        ica = _ion_ica;
        cai = _ion_cai;
        ...
   }

  Note that cao variable used/updated here is not ion variable but static one
  defined in the file scope.

  I believe this is not what user "expected" here.

• As we see this pattern in multiple mod files (including glia__dbbs_mod_collection__cdp5__CAM_GoC.mod
  mentioned in Testing & Fixing issues with DBBS-Lab's MOD file collection BlueBrain/nmodl#888), in this PR we disable
  declaring ion variables as CONSTANT. With this PR, we will get an error like:

   $ ./bin/nocmodl /.../nmodldb/models/db/modeldb/266799/mod/cdp_spiny.mod
   ...
   cao used in USEION statement can not be re-declared in the CONSTANT block at line 299 in file /.../nmodldb/models/db/modeldb/266799/mod/cdp_spiny.mod

@nrnhines : do you agree that explicit error like above is better approach than silently generating incorrect/undesired code? 🤔

[codecov-commenter]

Codecov Report

Merging #1955 (e4d57da) into master (1b76917) will increase coverage by 0.00%.
The diff coverage is 93.33%.

@@           Coverage Diff           @@
##           master    #1955   +/-   ##
=======================================
  Coverage   46.50%   46.50%           
=======================================
  Files         526      526           
  Lines      119216   119231   +15     
=======================================
+ Hits        55439    55453   +14     
- Misses      63777    63778    +1     

Impacted Files	Coverage Δ
src/nmodl/nocpout.cpp	93.25% <92.85%> (-0.01%)	⬇️
src/nmodl/modl.cpp	67.92% <100.00%> (+0.30%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[azure-pipelines]

✔️ 690ccab -> Azure artifacts URL

[pramodk]

@nrnhines : do you agree that explicit error like above is better approach than silently generating incorrect/undesired code? 🤔

@nrnhines : what do you think?

[nrnhines]

The explicit error is better. Thanks.

[pramodk]

ok good! I think there might be some mod files in the ModelDB. Before merging this PR, I can quickly check this in ModelDB and maybe fix this.

[azure-pipelines]

✔️ 0462052 -> Azure artifacts URL

[pramodk]

There are 3 models with 8 mod files where this pattern was used. It seems like all those mod files are same / similar and originated from the same implementation (filename with cdp*.mod).

I have created PRs here:

• Updated MOD file to fix the issue with the upcoming NEURON 9.0 release ModelDBRepository/244679#1
• Updated MOD file to fix the issue with the upcoming NEURON 9.0 release ModelDBRepository/265584#1
• Updated MOD file to fix the issue with the upcoming NEURON 9.0 release ModelDBRepository/243446#1

[azure-pipelines]

✔️ 4afde67 -> Azure artifacts URL

[olupton]

There are 3 models with 8 mod files where this pattern was used. It seems like all those mod files are same / similar and originated from the same implementation (filename with cdp*.mod).

I have created PRs here:
* ModelDBRepository/244679#1
* ModelDBRepository/265584#1
* ModelDBRepository/243446#1

I approved these, I believe that @ramcdougal needs to do something manual to make sure that the changes are propagated properly after the PRs are merged.

[alexsavulescu]

@pramodk looking at the modeldb-ci nightly there is also 138382. I commented out the cao CONSTANT assignment but now I see:

cdp3.cpp:2144:3: error: ‘cao’ was not declared in this scope; did you mean ‘ca0’?
 2144 |   cao = _ion_cao;
      |   ^~~
      |   ca0
cdp5.cpp: In function ‘void _ode_spec(NrnThread*, Memb_list*, int)’:
cdp5.cpp:1611:3: error: ‘cao’ was not declared in this scope; did you mean ‘ca0’?
....

[pramodk]

@pramodk looking at the modeldb-ci nightly there is also 138382. I commented out the cao CONSTANT assignment but now I see:

Should be fixed by ModelDBRepository/138382#1

[alexsavulescu]

thanks. updated to fix cdp5.mod in the same manner