New NMODL keyword for random variables. NEURON { RANDOM ranvar }#2627
Merged
Conversation
cp ../external/coding-conventions/cpp/clang-format-10 ../.clang-format clang-format-11 -i ./src/nrnoc/netstim.cpp
…format" This reverts commit d73f8a2.
cp ../external/coding-conventions/cpp/clang-format-10 ../.clang-format clang-format-11 -i ./src/nrnoc/netstim.cpp
1. variables are properly declared and registered 2. necessary functions are declared, defined and registered 3. able to call the functions from python world 4. add unit test to the same
- added src/nrnoc/netstim_rng.mod so that existing tests can work and we have a "cleaned" MOD file for RANDOM - have basic support implemented for setseq, init, sample methods with RANDOM variable - RANDOM variables are registered with "random" semantics but they are now treated as POINTER variables - revert src/nrnoc/netstim.mod to origin/master - remove temporary generated file cmake-build-debug/src/nrnoc/netstim.cpp
(but not getting error when owning handle goes out of existence)
pramodk
reviewed
Dec 6, 2023
pramodk
left a comment
Member
There was a problem hiding this comment.
Overall, looks OK. I will copy the comments that I just posted over Slack so that we have those here on the review.
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.
|
✔️ 8061d28 -> Azure artifacts URL |
This comment has been minimized.
This comment has been minimized.
|
✔️ a8dab46 -> Azure artifacts URL |
This comment has been minimized.
This comment has been minimized.
pramodk
added a commit
to BlueBrain/nmodl
that referenced
this pull request
Feb 8, 2024
…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>
This comment has been minimized.
This comment has been minimized.
|
✔️ 2197723 -> Azure artifacts URL |
|
|
✔️ e38cd81 -> Azure artifacts URL |
Collaborator
pramodk
approved these changes
Feb 8, 2024
pramodk
left a comment
Member
There was a problem hiding this comment.
LGTM - I believe multiple people have already looked and agreed current state is good to get this flying! 🚀
pramodk
added a commit
that referenced
this pull request
Mar 19, 2024
- after RANDOM construct (#2627) was merged, netstim.mod uses Random123 by default - before this, netstim was using scoprand by default and reference results were from the same - now, explicitly use Random123 in the test and update reference results
ohm314
pushed a commit
to BlueBrain/nmodl
that referenced
this pull request
May 21, 2024
…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>
ramcdougal
added a commit
that referenced
this pull request
May 31, 2024
* initial partial API * nrnpy_set_pr_etal should be extern C * nrn_call_function leaves results on stack * segments, properties * including hocdec.h to try to avoid Datum errors on some systems * sectionlist and allsec hoc_Item getters * symbol and object introspection * symbol table getting and interating * object ref, push * Renamed files to avoid having two nrnapi.h files. On some systems (most? but not the one I used for everything else), some files tried to include the wrong nrnapi.h during the "make install" phase. * neuronapi.cpp no longer uses its .h, that is now a header for 3rd party tools * switched to enum for stack types * needed typedefs for C to be happy * test: sections.c * clang-format * replaced hoc_Item with new synonym nrn_Item * netcon example shows no need for dlsym * support for .so files in addition to .dylib * removed an excess const We'll still treat the pointer as const, but the user has no reason to care. * Add Robert's API tests to CTest (#2365) * build + run tests * fixups * different fixup * add -rdynamic * bugfix flagged by ASan * fix to header file name of api * hh_sim and sections work with handles don't have quantitative identity with the old hh_sim results, but very close. * rename stack_types_t per @olupton * replace param_handle with param * working vclamp * unified API property functions * getters use const args * forgot to commit sections * vector_capacity now on const obj * Several improvements - C/C++ api header included in implementation - signatures made compatible - public nrn_Item defined as inheriting from hoc_Item - dropped overlapping definitions. types are both fw declarations and opaque types * Make tests link against nrniv_lib, no dlopen * Address sonar lint reservations * Make comparisons be part of the test and account for fp innacuraciesa * Tests to find ref files * api: Make tests resilient to accumulated errors * Use in-test ref result, depending on NRN_ENABLE_CORENEURON * Use free for allocated c strings * Dont alloc Section Item to heap as its not used. NOTE: It seems that Sections are never freed. However we are lacking API to free Sections and Symbols in general. * Address intel compiler complain * Free memb_func[].dparam_semantics * Switch dparam_semantics to unique_ptr (no raw!) * Memb_list to do its memory management * Memb_list to be aware of its potential "view" condition and not free * Back pdata free * nrn_init to better initialize mpi * Addressed a number of warnings, inc avoidiing potential double free * Addressed sonarcloud issues * fix the merge with master * Fix test/api/netcon.cpp after merge with master - after RANDOM construct (#2627) was merged, netstim.mod uses Random123 by default - before this, netstim was using scoprand by default and reference results were from the same - now, explicitly use Random123 in the test and update reference results * fix warning [-Wimplicit-exception-spec-mismatch] --------- Co-authored-by: Olli Lupton <oliver.lupton@epfl.ch> Co-authored-by: Fernando Pereira <fernando.pereira@epfl.ch> Co-authored-by: Pramod Kumbhar <pramod.s.kumbhar@gmail.com> Co-authored-by: Nicolas Cornu <nicolas.cornu@epfl.ch> Co-authored-by: Michael Hines <michael.hines@yale.edu>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



A simplified alternative to #2597
See the documentation as part of this PR. Basically the random variable specifies only the generator and its properties, not the distribution. Default
nrnran123_Stateproperties are unique per instance and can be set and get by HOC or Python.See BlueBrain/nmodl#1125
The new hoc syntax increases SHIFT/REDUCE conflicts from 96 to 108
nocmodl. When this is checked can remove nrn/netstim.cppCI_BRANCHES:NMODL_BRANCH=master,