Support for NEURON {RANDOM ranvar1, ranvar2, ...} statement. by nrnhines · Pull Request #1125 · BlueBrain/nmodl

nrnhines · 2023-12-15T00:55:17Z

Accepts the latest netcon.mod from neuronsimulator/nrn#2627. Some changes were picked from #581

Misuse of RANDOM variables should result in meaningful error messages
Unit tests
Semantic analysis visitor improvements

nrnhines · 2023-12-15T01:07:49Z

This comment is obsolete. nrn/netstim.cpp has been removed as the translation of RANDOM variables with nmodl is complete.

nrn/netstim.cpp is temporary and has been manually modified from the nmodl generated cpp file. The diff is below and
can be used to indicate how the nmodl translator needs to be augmented.

hines@hines-t7500:~/neuron/rannmodl/buildsani$ bin/nmodl ../src/nrnoc/netstim.mod
hines@hines-t7500:~/neuron/rannmodl/buildsani$ diff netstim.cpp ../netstim.cpp
7c7
< Created         : Sat Dec 16 19:30:54 2023
---
> Created         : Sat Dec 16 11:04:46 2023
46a47
>         "ranvar",
49a51
> #define Zranvar (nrnran123_State*)inst->ranvar[indexes[2*pnodecount + id]]
77a80
>         void** ranvar{};
96c99
<         return -1;
---
>         return 2;
111c114
<         return 3;
---
>         return 4;
164a168,175
> 
>         int pnodecount = ml->_nodecount_padded;
>         int nodecount = ml->nodecount;
>         Datum* indexes = ml->pdata;
>         for (int id = 0; id < nodecount; id++) {
>             nrnran123_deletestream(Zranvar);
>         }
> 
191a203
>         inst->ranvar = nt->_vdata;
266c278
<         random_setseq(ranvar, x);
---
>         nrnran123_setseq(Zranvar, x);
281c293
<                 nrnran123_setids(ranvar, id[0], id[1], id[2]);
---
>                 nrnran123_setids(Zranvar, id[0], id[1], id[2]);
284c296
<                 nrnran123_setseq(ranvar, id[0], which);
---
>                 nrnran123_setseq(Zranvar, id[0], which);
297c309
<                 nrnran123_setids(ranvar, static_cast<uint32_t>(*getarg(1)), static_cast<uint32_t>(*getarg(2)), static_cast<uint32_t>(*getarg(3)));
---
>                 nrnran123_setids(Zranvar, static_cast<uint32_t>(*getarg(1)), static_cast<uint32_t>(*getarg(2)), static_cast<uint32_t>(*getarg(3)));
299c311
<                 nrnran123_setids(ranvar, static_cast<uint32_t>(*getarg(1)), static_cast<uint32_t>(*getarg(2)), 0);
---
>                 nrnran123_setids(Zranvar, static_cast<uint32_t>(*getarg(1)), static_cast<uint32_t>(*getarg(2)), 0);
301c313
<             nrnran123_setseq(ranvar, 0, 0);
---
>             nrnran123_setseq(Zranvar, 0, 0);
324c336
<         ret_erand = random_negexp(ranvar);
---
>         ret_erand = nrnran123_negexp(Zranvar);
352c364
<                     artcell_net_send(&inst->tqitem[indexes[2*pnodecount + id]], weight_index, pnt, nt->_t+inst->event[id], 1.0);
---
>                     artcell_net_send(&inst->tqitem[indexes[3*pnodecount + id]], weight_index, pnt, nt->_t+inst->event[id], 1.0);
360c372
<                     artcell_net_send(&inst->tqitem[indexes[2*pnodecount + id]], weight_index, pnt, nt->_t+0.0, 1.0);
---
>                     artcell_net_send(&inst->tqitem[indexes[3*pnodecount + id]], weight_index, pnt, nt->_t+0.0, 1.0);
368c380
<                     artcell_net_send(&inst->tqitem[indexes[2*pnodecount + id]], weight_index, pnt, nt->_t+inst->event[id], 1.0);
---
>                     artcell_net_send(&inst->tqitem[indexes[3*pnodecount + id]], weight_index, pnt, nt->_t+inst->event[id], 1.0);
409c421
<                     artcell_net_send(&inst->tqitem[indexes[2*pnodecount + id]], 0, (Point_process*)inst->point_process[indexes[1*pnodecount + id]], nt->_t+inst->event[id], 3.0);
---
>                     artcell_net_send(&inst->tqitem[indexes[3*pnodecount + id]], 0, (Point_process*)inst->point_process[indexes[1*pnodecount + id]], nt->_t+inst->event[id], 3.0);
431c443,444
<         hoc_register_dparam_semantics(mech_type, 2, "netsend");
---
>         hoc_register_dparam_semantics(mech_type, 2, "random");
>         hoc_register_dparam_semantics(mech_type, 3, "netsend");
433c446
<         add_nrn_artcell(mech_type, 2);
---
>         add_nrn_artcell(mech_type, 3);
hines@hines-t7500:~/neuron/rannmodl/buildsani$

…function.

- replace possibly_rename() with lambda as it's used in one place - remove extdef_rand.h - move is_random_function from symtab/symbol_properties.hpp to visitor_utils.hpp - add parsing test in test/unit/utils/nmodl_constructs.cpp

pramodk

@nrnhines: tomorrow I am going to work a bit on src/visitors/semantic_analysis_visitor.* and address Todo about meaningful error message.

- fix failing modtoken tests - add units for semantic analysis visitor of RANDOM - simplify the logic for semantic checks - remove unnecessary code

codecov · 2024-02-08T00:23:26Z

Codecov Report

Attention: 22 lines in your changes are missing coverage. Please review.

Comparison is base (3e3f62a) 88.18% compared to head (2b8950d) 88.11%.
Report is 1 commits behind head on master.

Files	Patch %	Lines
src/codegen/codegen_cpp_visitor.cpp	44.44%	10 Missing ⚠️
src/codegen/codegen_coreneuron_cpp_visitor.cpp	50.00%	8 Missing ⚠️
src/visitors/perf_visitor.cpp	57.14%	3 Missing ⚠️
src/symtab/symbol_properties.cpp	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1125      +/-   ##
==========================================
- Coverage   88.18%   88.11%   -0.08%     
==========================================
  Files         175      175              
  Lines       12411    12520     +109     
==========================================
+ Hits        10945    11032      +87     
- Misses       1466     1488      +22

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

bbpbuildbot · 2024-02-08T01:29:09Z

Logfiles from GitLab pipeline #191849 (:white_check_mark:) have been uploaded here!

Status and direct links:

pramodk

LGTM - I have made the necessary changes directly.

…tatement (#1125) * Support for new RANDOM construct in NMODL, see neuronsimulator/nrn#2627 * Added necessary changes to the parser, visitors, symbol table and code generation * Added unit tests Co-authored-by: Pramod S Kumbhar <pramod.s.kumbhar@gmail.com>

…tatement (BlueBrain/nmodl#1125) * Support for new RANDOM construct in NMODL, see neuronsimulator/nrnBlueBrain/nmodl#2627 * Added necessary changes to the parser, visitors, symbol table and code generation * Added unit tests Co-authored-by: Pramod S Kumbhar <pramod.s.kumbhar@gmail.com> NMODL Repo SHA: BlueBrain/nmodl@8f7eb99

nrnhines marked this pull request as draft December 15, 2023 00:55

This comment has been minimized.

Sign in to view

nrnhines mentioned this pull request Dec 17, 2023

New NMODL keyword for random variables. NEURON { RANDOM ranvar } neuronsimulator/nrn#2627

Merged

12 tasks

This comment has been minimized.

Sign in to view

nrnhines commented Jan 5, 2024

View reviewed changes

Comment thread src/codegen/codegen_coreneuron_cpp_visitor.cpp Outdated

Comment thread src/visitors/semantic_analysis_visitor.cpp Outdated

Comment thread src/visitors/semantic_analysis_visitor.cpp Outdated

Comment thread src/visitors/semantic_analysis_visitor.cpp Outdated

This comment has been minimized.

Sign in to view

nrnhines and others added 7 commits January 27, 2024 13:48

Incomplete implementation of RANDOM statement.

816f7b1

NMODL RANDOM netstim.mod translates correctly into netstim.cpp

6862ca7

User error if random_var used anywhere except as first arg of random …

e288a8e

…function.

location information when random_var errors

27c9ae4

unordered_map not included on some macs

b5e065a

Also expose random_dblpick()

9384ec0

Rename random_dblpick() to random_dpick()

c89dcf1

pramodk force-pushed the hines/nmodlrandom-simplified branch from d4efc03 to c89dcf1 Compare January 27, 2024 13:56

This comment has been minimized.

Sign in to view

nrnran123_setseq1 renamed to nrnran123_setseq

cabe5da

This comment has been minimized.

Sign in to view

Merge branch 'master' into hines/nmodlrandom-simplified

c3c4740

This comment has been minimized.

Sign in to view

Some improvements from code review

f470a32

- replace possibly_rename() with lambda as it's used in one place - remove extdef_rand.h - move is_random_function from symtab/symbol_properties.hpp to visitor_utils.hpp - add parsing test in test/unit/utils/nmodl_constructs.cpp

pramodk reviewed Feb 6, 2024

View reviewed changes

Comment thread src/visitors/semantic_analysis_visitor.cpp Outdated

Comment thread src/codegen/codegen_coreneuron_cpp_visitor.cpp Outdated

This comment has been minimized.

Sign in to view

Various improvements

20b6e94

- fix failing modtoken tests - add units for semantic analysis visitor of RANDOM - simplify the logic for semantic checks - remove unnecessary code

This comment has been minimized.

Sign in to view

avoid use of nrnran123_State* as variable type. Use void*

2b8950d

pramodk changed the title ~~NEURON {RANDOM ranvar1, ranvar2, ...} statement.~~ Support for NEURON {RANDOM ranvar1, ranvar2, ...} statement. Feb 8, 2024

pramodk marked this pull request as ready for review February 8, 2024 01:01

pramodk approved these changes Feb 8, 2024

View reviewed changes

pramodk merged commit 8f7eb99 into master Feb 8, 2024

pramodk deleted the hines/nmodlrandom-simplified branch February 8, 2024 07:03

alkino mentioned this pull request Oct 22, 2024

Better support for Random123 routines in verbatim blocks for GPU #130

Open

Uh oh!

Conversation

nrnhines commented Dec 15, 2023 • edited by pramodk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nrnhines commented Dec 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

pramodk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

codecov Bot commented Feb 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

This comment has been minimized.

bbpbuildbot commented Feb 8, 2024

Uh oh!

pramodk left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nrnhines commented Dec 15, 2023 •

edited by pramodk

Loading

nrnhines commented Dec 15, 2023 •

edited

Loading

codecov Bot commented Feb 8, 2024 •

edited

Loading