Codecov Report

Attention: Patch coverage is 58.74126% with 59 lines in your changes missing coverage. Please review.

Project coverage is 47.16%. Comparing base (2c3a21d) to head (8c0bfcf).
Report is 1828 commits behind head on master.

@@            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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests

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.

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.

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

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

✔️ c197fc1 -> Azure artifacts URL

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

✔️ 6d656ac -> Azure artifacts URL

✔️ 1d077e8 -> Azure artifacts URL

✔️ 802603e -> Azure artifacts URL

✔️ 802603e -> Azure artifacts URL

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

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

✔️ 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 ✅.

✔️ 0f73f47 -> Azure artifacts URL

✔️ 8c0bfcf -> Azure artifacts URL

🚀