Fix implicit argument handling#1325
Conversation
`at_time` requires an instance of `NrnThread*`, which is implicit in the mod file.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1325 +/- ##
=======================================
Coverage 85.37% 85.37%
=======================================
Files 178 178
Lines 13458 13458
=======================================
Hits 11490 11490
Misses 1968 1968 ☔ View full report in Codecov by Sentry. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Please check how widely |
|
@1uc let's just say the answer is "non-zero". Can we change coreNEURON codegen instead then (i.e. use option 2 from the description)? |
|
You might want to check neurodamus-models before changing the tests, BBP has written MOD files that consist mostly of VERBATIM blocks. I assume they work with CoreNEURON (and NEURON). Therefore, there's a good chance that they use both |
|
Using both |
|
Logfiles from GitLab pipeline #218158 (:white_check_mark:) have been uploaded here! Status and direct links: |
at_time, an internal NEURON function, requires an instance ofNrnThread*, which is implicit in the mod file, but was previously called with a non-existing argument name (parent function hadNrnThread* _nt, while the call toat_timehad passednt).Note that I am slightly annoyed at the possible fixes here: in NEURON codegen, all of the arguments passed to functions apparently start with an underscore, while for coreNEURON codegen all of them start without an underscore.
This suggests a couple of possible fixes:
ntinstead of_nt(this PR)_ntinstead ofnt(requires a bit more work since the unit tests check the codegen directly)ImplicitArgumentVisitor(or make another one, or add a function in the codegen) which correctly sets the right variable (either with or without an underscore) depending on the the code backend used for the codegen