Testing & Fixing issues with DBBS-Lab's MOD file collection

[pramodk]

We have been working with Robin in order to run his model with CoreNEURON on GPU (see neuronsimulator/nrn/issues/1839). While testing the model, we have seen quite some issues in NMODL translation. The goal of this ticket is to compile mod files and fix translation+compilation issues.

What to do?

• Install coreneuron + nmodl with sympy (-DCORENRN_ENABLE_NMODL=ON "-DCORENRN_NMODL_FLAGS=sympy --analytic")
• Get mod file collection from DBBS Lab

git clone https://github.com/dbbs-lab/dbbs-mod-collection.git

Few mod file has issues and we should do following:

1. following file is not required and incompatible with coreneuron

rm dbbs-mod-collection/dbbs_mod_collection/mod/glia__dbbs_mod_collection__gap_junction__parallel.mod 

2. dbbs_mod_collection/mod/glia__dbbs_mod_collection__cdp5__CR.mod file needs following change (state variable can't be in COMPARTMENT)

diff --git a/dbbs_mod_collection/mod/glia__dbbs_mod_collection__cdp5__CR.mod b/dbbs_mod_collection/mod/glia__dbbs_mod_collection__cdp5__CR.mod
index 32df989..7edb9ef 100644
--- a/dbbs_mod_collection/mod/glia__dbbs_mod_collection__cdp5__CR.mod
+++ b/dbbs_mod_collection/mod/glia__dbbs_mod_collection__cdp5__CR.mod
@@ -100,6 +100,7 @@ ASSIGNED {
        cai       (mM)
        mgi     (mM)
        vrat    (1)
+       mg              (mM)^M
 }

 CONSTANT { cao = 2     (mM) }
@@ -110,7 +111,6 @@ STATE {
        : let it be ~1.5 - 2 orders of magnitude smaller than baseline level

        ca              (mM)    <1e-3>
-       mg              (mM)    <1e-6>

        Buff1           (mM)
        Buff1_ca        (mM)
@@ -223,7 +223,7 @@ LOCAL dsq, dsqvol  : can't define local variable in KINETIC block
                    :   or use in COMPARTMENT statement

 KINETIC state {
-  COMPARTMENT diam*diam*vrat {ca mg Buff1 Buff1_ca Buff2 Buff2_ca BTC BTC_ca DMNPE DMNPE_ca CR CR_1C_0N CR_2C_0N CR_2C_1N CR_0C_1N CR_0C_2N CR_1C_2N CR_1C_1N CR_2C_1N CR_1C_2N CR_2C_2N}
+  COMPARTMENT diam*diam*vrat {ca Buff1 Buff1_ca Buff2 Buff2_ca BTC BTC_ca DMNPE DMNPE_ca CR CR_1C_0N CR_2C_0N CR_2C_1N CR_0C_1N CR_0C_2N CR_1C_2N CR_1C_1N CR_2C_1N CR_1C_2N CR_2C_2N}^M
   COMPARTMENT (1e10)*parea {pump pumpca}

• With above two changes, mod files compile with mod2c but not with NMODL. So our goal is to make sure mod files can be compiled with NMODL as well i.e.

nrnivmodl-core dbbs-mod-collection/dbbs_mod_collection/mod/

Fixed

• glia__dbbs_mod_collection__cdp5__CAM_GoC.mod : verify if LOCAL and CONSTANT are correctly translated : @pramodk handled this via Semantic analysis check for USEION variables in CONSTANT block #908
• glia__dbbs_mod_collection__Cav2_3__0.mod : TABLE statement with ARRAY variables : fixed by Code generation fixes for array variables in TABLE statement #924
• glia__dbbs_mod_collection__Ca__granule_cell.mod: TABLE statement from DERIVATIVE block with Eigen : fixed by Codegen fixes for Eigen solvers with use of PROCEDURE using TABLE statement #925
• glia__dbbs_mod_collection__Nav1_1__0.mod : fixed by Improve Eigen solver node in the ast to allow local variables and conditional expressions #89

 x86_64/corenrn/mod2c/glia__dbbs_mod_collection__Nav1_1__0.cpp:191:30: error: duplicate member 'g'
         double* __restrict__ g{};
                              ^
 x86_64/corenrn/mod2c/glia__dbbs_mod_collection__Nav1_1__0.cpp:125:30: note: previous declaration is here
         double* __restrict__ g{};                              ^
 1 error generated.

Typically, g is implicitly defined in the MOD file. In this MOD file, g is declared as a RANGE variable and this causes double member declaration in the instance structure.

     struct glia__dbbs_mod_collection__Nav1_1__0_Instance  {
         double* __restrict__ celsius{&coreneuron::celsius};
         const double* __restrict__ gbar{};
         double* __restrict__ ina{};
         double* __restrict__ i{};
         double* __restrict__ igate{};
         double* __restrict__ g{};
..
         double* __restrict__ DI6{};
         double* __restrict__ g{};
...

To be fixed

• glia__dbbs_mod_collection__cdp5__0.mod,
  glia__dbbs_mod_collection__cdp5__CAM_GoC.mod
glia__dbbs_mod_collection__cdp5__CAM.mod,
  glia__dbbs_mod_collection__cdp5__CR.mod

Take updated mod files from dbbs-lab/dbbs-mod-collection#6.

Above mod files define LOCAL variables at global scope as:

 LOCAL dsq, dsqvol  : can't define local variable in KINETIC block
                    :   or use in COMPARTMENT statement

 KINETIC state {
   COMPARTMENT diam*diam*vrat {ca Buff1 Buff1_ca Buff2 Buff2_ca BTC BTC_ca DMNPE DMNPE_ca CR CR_1C_0N CR_2C_0N CR_2C_1N CR_0C_1N CR_0C_2N CR_1C_2N CR_1C_1N CR_2C_1N CR_1C_2N   CR_2C_2N}
   COMPARTMENT (1e10)*parea {pump pumpca}


     :pump
     ~ ca + pump <-> pumpca  (kpmp1*parea*(1e10), kpmp2*parea*(1e10))
     ~ pumpca <-> pump   (kpmp3*parea*(1e10), 0)
     CONSERVE pump + pumpca = TotalPump * parea * (1e10)

     ica_pmp = 2*FARADAY*(f_flux - b_flux)/parea
     : all currents except pump
     : ica is Ca efflux
     ~ ca << (-ica*PI*diam/(2*FARADAY))

and when we compile generated .cpp file then we get:

 x86_64/corenrn/mod2c/glia__dbbs_mod_collection__cdp5__CR.cpp:731:2053: error: use of undeclared identifier 'dsqvol'
 x86_64/corenrn/mod2c/glia__dbbs_mod_collection__cdp5__CR.cpp:700:21: error: use of undeclared identifier 'dsq'
                     dsq = inst->diam[indexes[4*pnodecount + id]] * inst->diam[indexes[4*pnodecount + id]];
                     ^
 x86_64/corenrn/mod2c/glia__dbbs_mod_collection__cdp5__CR.cpp:701:21: error: use of undeclared identifier 'dsqvol'
                     dsqvol = dsq * inst->vrat[id];                     ^
 x86_64/corenrn/mod2c/glia__dbbs_mod_collection__cdp5__CR.cpp:701:30: error: use of undeclared identifier 'dsq'
                     dsqvol = dsq * inst->vrat[id];

It's true that we can not use a LOCAL variable as it's not thread/SIMD safe (and we have a special pass to convert LOCAL to RANGE). But, I am expecting NMODL to show correct error than resulting in incorrect code and then compilation error?

Note: For now we can manually change LOCAL variables to RANGE and have mod file compiled. But, the generated Eigen code seems scary (in terms of complexity) and we need to involve @cattabiani to take a look. See #930.

• glia__dbbs_mod_collection__Na__granule_cell_FHF.mod and glia__dbbs_mod_collection__Na__granule_cell.mod

These two files have function calls in the KINETIC block, see #927.

 KINETIC kstates {
     : 1 riga
     ~ C1 <-> C2 (n1*alfa(v),n4*beta(v))
     ~ C2 <-> C3 (n2*alfa(v),n3*beta(v))

Note: Temporarily, one could re-write the mod file by first calling function calls before equations and storing the returned values into LOCAL variables?

[Helveg]

Also, to us this is brittle legacy code, if you have any advice/sanitizations to make these mod files more standard; there's likely no technical reason for the weirder things they do, other than that they are 20 years old and noone knows better.

[pramodk]

@Helveg : thanks! Definitely revisit them. Some of the MOD files can be made compatible with some changes but I also wanted to make NMODL to handle all legacy usage patterns. Hence would like to fix the issues.

We will come back to you once few remaining issues are solved.