Compile NEURON mechanisms as C++

[olupton]

Overview

There has been a long-term effort to migrate NEURON from C to C++, including efforts such as #763.

Most NEURON source code files are already compiled as C++, two significant exceptions to this are:

• The code generated from .mod files by the nocmodl transpiler
• Some bundled quasi-external libraries, such as mesch, scopmath, sparse13 and sundials.

This PR aims to address the first one of these and generate C++ from nocmodl.

This constitutes progress towards part 2 of #708, and it also encompasses #384 and makes #397 obsolete.

Several other PRs have already been extracted from this one and merged: #1598, #1604, #1606, #1609.

Approach

The strategy of this PR has been, broadly, to:

• Leave things alone if they don't break.
• Replace .c with .cpp and C with CXX throughout the relevant parts of the build system.
• When things do break, remove extern "C" and prefer C++ linkage -- this allows overloading.
  • Important point: a C-style declaration void consume_an_int(); is interpreted as declaring an overload that takes no arguments, and a subsequent call consume_an_int(42) will call the void consume_an_int(int); overload that was declared in an included header. This doesn't always work, as we cannot overload on return type.
  • Exception: symbols that are used in code that is still C (stoprun, mcell_ran4_legacy)
  • Exception: symbols that get used with dlsym (modl_reg)
• Prefer declarations in header files, tend to remove explicit declarations in source files referring to methods in other source files.
  • Related: introduce a mech_api.h header that defines the NEURON API that may be used in .mod files.
  • In the spirit of trying to leave things alone that aren't broken, nocmodl still emits a lot of declarations instead of including headers.
• Introduce [[deprecated]] overloads that mean legacy type-unsafe patterns that are common in VERBATIM blocks are still valid. Example:

int vector_capacity(IvocVect*);
[[deprecated("non-void* overloads are preferred")]] int vector_capacity(void*);

• Bump the required C++ standard to C++14 ([[deprecated]], ...)
• Introduce an off-by-default utility to try and manipulate common patterns in .mod files that are invalid C++ to give valid generated code (nrnivmodl -legacytransformations cxx, nocmodl -t cxx)
  • This is not used for .mod files bundled with NEURON. The .mod changes in this PR are similar to the transformations made by the automated utility, but not identical (the bundled .mod files additionally avoid all [[deprecated]] methods).
• Update nrntest to include Drop obsolete declarations. nrntest#26

Status

In my local NEURON + CoreNEURON + NMODL + GPU build then all tests have passed. See #1597 (comment) and #1597 (comment) for updates on the status of the ModelDB CI (https://github.com/neuronsimulator/nrn-modeldb-ci/) when run against these changes. Roughly 33/675 models remain incompatible with C++.

TODO:

• Merge Update tests model for MOD to C as well as C++ translation #1747 with updates to the test models that make them dual C/C++ compatible
• Check compatibility of internal BBP models via CoreNEURON CI.
• Check ModelDB CI again. This should include some combination of -fpermissive and -legacytransformations cxx for at least some of the models, so naively triggering the GitHub Actions workflow is not going to be enough.
• Feedback on documentation after changes referred to by Update tests model for MOD to C as well as C++ translation #1747
• Update CoreNEURON submodule commit after Bump semantic version to 9.0.0 BlueBrain/CoreNeuron#828.
• Add a link to the C++ migration documentation if there is an error, or open a follow up issue for this.

cc: @alexsavulescu

[codecov-commenter]

Codecov Report

Merging #1597 (8c0bfcf) into master (2c3a21d) will decrease coverage by 0.00%.
The diff coverage is 58.74%.

@@            Coverage Diff             @@
##           master    #1597      +/-   ##
==========================================
- Coverage   47.17%   47.16%   -0.01%     
==========================================
  Files         543      543              
  Lines      112957   112964       +7     
==========================================
- Hits        53287    53284       -3     
- Misses      59670    59680      +10     

Impacted Files	Coverage Δ
src/ivoc/graph.cpp	10.39% <ø> (ø)
src/ivoc/ivocmain.cpp	74.85% <ø> (ø)
src/ivoc/ivocvect.h	95.12% <ø> (ø)
src/ivoc/matrix.cpp	51.57% <0.00%> (-0.14%)	⬇️
src/ivoc/xmenu.cpp	3.68% <ø> (ø)
src/modlunit/units.cpp	77.32% <ø> (ø)
src/nmodl/simultan.cpp	68.52% <0.00%> (-1.06%)	⬇️
src/nrncvode/cvodeobj.cpp	85.97% <ø> (ø)
src/nrncvode/hocevent.cpp	72.58% <ø> (ø)
src/nrniv/bbsavestate.h	50.00% <ø> (ø)
... and 51 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[olupton]

As a reference, the current master on my test system, using @alexsavulescu’s https://github.com/neuronsimulator/nrn-modeldb-ci/ has 5 build/link failures from 680 models, and 1 more runtime failure.

The 5 are numbers 3658 97868 20212 144549 186768; I am putting these aside for now.

With the current state of this PR, I have compilation errors from 50 of the remaining 675 models:

259732 189154 35358 184725 12631 187604 62673 139421 195615 37856 123453 223891 182129 141505 139150 185123 138379 168874 52034 113732 136176 136095 185858 150245 241240 105507 37819 153280 7399 144538 150240 241165 136310 146949 146565 244262 140881 124394 26997 97874 144586 151282 106891 124043 116862 116830 189347 7400 9889 116838

I should note that I added -fpermissive for this, which may be pragmatic for old models but should certainly not be a new default.

At risk of not being very helpful, filenames from these models with compilation errors are:

GABALOW.cpp intf_.cpp GABAB1.cpp repeatconn.cpp stats.cpp intf6_.cpp updown.cpp bipolarNMDA.cpp clampex.cpp ribbon1.cpp rand.cpp ampa_syn.cpp ampa.cpp vecst.cpp linsyn.cpp infot.cpp gabaA.cpp Ksyn.cpp nmda_syn.cpp jitcon.cpp precall.cpp ProbGABAAB_EMS.cpp SquareInput.cpp inhib.cpp staley.cpp double_synapse.cpp ProbGABAAB_EMS_GEPH_g.cpp intf6.cpp glutamate.cpp ImpedanceFM.cpp mcna1e.cpp fastconn.cpp nstim.cpp NMDA_Mg_T.cpp misc.cpp bnet.cpp AMPA.cpp field.cpp RandomGenerator.cpp VSFP31M3.cpp holt_rnd.cpp

Looking through at the failures, I would summarise the causes as:

1. Cases like in https://senselab.med.yale.edu/ModelDB/ShowModel?model=105507&file=/b07dec17/misc.mod, where we have FILE* f, *hoc_obj_file_arg(); and then hoc_obj_file_arg(2);. The local declaration with no arguments shadows the correct prototype that is declared in the header https://github.com/neuronsimulator/nrn/blob/74956640f57b10f2da8547ad5f1565b40ad2f765/src/oc/oc_ansi.h#L74 This can be avoided with an extension of this logic: https://github.com/neuronsimulator/nrn/blob/74956640f57b10f2da8547ad5f1565b40ad2f765/src/nmodl/modl.cpp#L382-L394, but this is clearly a last resort. hoc_call_func appears to be another example of this, as is hoc_objgetarg.
2. Cases like betacf and gammln in https://senselab.med.yale.edu/ModelDB/ShowModel?model=105507&file=/b07dec17/stats.mod#tabs-2, where a function internal to a mechanism is declared without the correct prototype. Other examples: ismono1, list_vector_px, list_vector_resize, gsort2, vector_newsize, scrset, openvec, hashseed2, uniq2, IsList, AllocILV, AllocListVec, FreeListVec, vprpr, getlp, lop, ishuffle, pdfpr
3. Use of C++ keywords as variable names, such as template in https://github.com/neuronsimulator/nrn/blob/74956640f57b10f2da8547ad5f1565b40ad2f765/src/oc/hocdec.h#L233-L237 being referred to in mechanisms such as https://senselab.med.yale.edu/ModelDB/ShowModel?model=105507&file=/b07dec17/vecst.mod#tabs-2 (method isojt) and new being used as a variable name in https://senselab.med.yale.edu/ModelDB/ShowModel?model=106891&file=/b07dec27_20091025/vecst.mod#tabs-2, and true as a variable name in https://senselab.med.yale.edu/ModelDB/ShowModel?model=141505&file=/neuroprosthesis/staley.mod#tabs-2.
4. Use of obsolete K&R syntax, as described in Converting NEURON source to fully C++ codebase #708 (comment). Other method names seen: get_parm, header_endian, reverse, __uitrunc, shake, calc_impedances, my_srand48, holt_normrand
5. NEURON API methods that do not (yet) have compatibility wrappers defined: mcell_ran4, nrn_event_queue_stats, state_discontinuity, nrnRan4int
6. Invalid C++ syntax as a result of more restrictive casting rules; for example: int *pids; unsigned long *pcombids; pcombids=pids=0x0.
7. Possible bug (introduced?) in nocmodl: duplicate declaration of _coef1 in model 62673 file mcna1e.cpp and model 123453 file VSFP31M3.cpp and model 223891 file NMDA_Mg_T.cpp, this seems to have been a problem in the code generation, patched up with bdb9ed5
8. Possible bug (introduced?) in nocmodl: handling of ResetSeed procedure in RandomGenerator.mod, model 150240, this seems to be a problem in the mechanism, which declares ResetSeed with no arguments and then calls it with one in the INITIAL block
9. Possible bug (introduced?) in nocmodl: naming of state method in Ksyn.mod, model 37856. This was introduced by a change I have now reverted again.
10. System calls without inclusions, e.g. getpid in model 138379 and 140881 intf6_.cpp, sqrt…
11. System calls with wrong numbers of arguments, e.g. sleep_ in model 26997 vecst.mod
12. Attempt to call NEURON method scop_random with arguments when it is declared void, ProbGABAAB_EMS_GEPH_g.mod, model 182129 and ProbGABAAB_EMS.mod model 241165

I think (1), (5), (7-9) deserve slightly more attention; some change to this PR may help. The others seem nearly hopeless without modifying the models themselves.

As you can see, the current PR passes the CI we have running on GitHub.

[olupton]

I re-ran https://github.com/neuronsimulator/nrn-modeldb-ci/ against the latest version of this branch, and now have a total of 38 failures from 680 models. As noted above, 5 of the 680 models did not build with master anyway, so it seems that 33/675 remain incompatible with this changeset.

I don't propose to repeat the exercise of trawling the logs and updating #1597 (comment) -- a summary of the change from the status then is:

• (1): this logic has been extended a bit, but now sits behind a new option to nocmodl and nrnivmodl. At present, that option is used when building the nrn-modeldb-ci and for some of the external tests in NEURON, but not for any of the mechanisms that are shipped with NEURON itself. The tentative idea here is that we would update the external tests included in the CTest suite to not need the option any more, but that it could remain as a compatibility measure for old models in the database.
• (5): these methods were updated on the NEURON side in this PR and should now work
• (7): this was patched in nocmodl as part of this PR
• (8) and (9): these were introduced in an earlier version of this PR and are now gone again.

The other causes listed above remain valid and are (at least mainly) responsible for the 33 remaining failures.

[olupton]

https://bbpgitlab.epfl.ch/hpc/coreneuron/-/pipelines/35356 is testing this in the CoreNEURON pipeline with the Intel and NVIDIA compilers. Update: this passed.

[olupton]

BlueBrain/CoreNeuron#770 is testing this against some more BBP models. As shown in BlueBrain/CoreNeuron#770 (comment), they now build and run.

[nrnhines]

Fix failing nrn-modeldb-ci models on windows.

In summary, the bulk of the models below that have not been successfully built with this PR are those that use drand48 and related random functions that linux declares in stdlib.h. There are several ways to repair this, but they don't really pertain to this PR. Although it's possible that another substantive issue is hidden in a subsequent mod file after the nrnivmodl failure, that seems unlikely as all these are from the same family. So remaining nrn-modeldb-ci issues on windows should not delay merging this PR.

• 7399 replace some (unsigned long) with (size_t)
• 12631 random and initstate not available from stdlib.h, could replace with 9889/rand.mod
• 9889 use fixed but otherwise indentical 12631 presyn.inc
• 37819 needs drand48 and srand48 (could use the private versions in 9889/rand.mod)
• 52034 needs srand48
• 97868 needs mysql
• 105507 needs srand48
• 106891 needs srand48
• 113732 fixed with (size_t)
• 116830 needs drand48
• 116838 needs drand48
• 116862 undef and redefine t around #include "misc.h" due to c:/nrn/mingw/mingw64/include/pthread.h
• 124291 get rid of VERBATIM return 0...
• 136095 needs srand48
• 138379 needs srand48
• 139421 needs srand48
• 140881 srand48
• 141505 srand48
• 144538 srand48
• 144549 srand48
• 144586 srand48
• 146949 srand48
• 150245 srand48
• 151282 srand48
• 153280 uint32_t replaces u_int32_t
• 168874 srand48
• 185858 undef and redefine t around #include "misc.h"
• 186768 srand48
• 187604 uint32_t replaces u_int32_t
• 189154 srand48
• 195615 undef and redefine t around #include "misc.h". Also no such thing as SIGHUP
• 262115 random and srandom not declared in this scope

[azure-pipelines]

✔️ 6d656ac -> Azure artifacts URL

[azure-pipelines]

✔️ 1d077e8 -> Azure artifacts URL

[azure-pipelines]

✔️ 802603e -> Azure artifacts URL

[olupton]

✔️ 802603e -> Azure artifacts URL

I just launched https://github.com/neuronsimulator/nrn-modeldb-ci/actions/runs/2515695442 using this URL and neuron-nightly.

[olupton]

I also launched the CoreNEURON pipeline against this branch of NEURON: https://bbpgitlab.epfl.ch/hpc/coreneuron/-/pipelines/61107 (internal link).

[olupton]

✔️ 802603e -> Azure artifacts URL

I just launched https://github.com/neuronsimulator/nrn-modeldb-ci/actions/runs/2515695442 using this URL and neuron-nightly.

This looks fine ✅.

[azure-pipelines]

✔️ 0f73f47 -> Azure artifacts URL

[azure-pipelines]

✔️ 8c0bfcf -> Azure artifacts URL

[alexsavulescu]

🚀