WIP: Support for RANDOM construct in NMODL#2597
Closed
pramodk wants to merge 15 commits into
Closed
Conversation
This comment has been minimized.
This comment has been minimized.
a05183c to
04ca7e3
Compare
This comment has been minimized.
This comment has been minimized.
ohm314
reviewed
Nov 1, 2023
|
|
||
| // TODO: keep track of arguments for RANDOM | ||
| // could be removed by fixing production rules? | ||
| static std::vector<std::string> numbers; |
Member
There was a problem hiding this comment.
a bit astonished this is necessary - you don't have argument lists anywhere else?
Comment on lines
+158
to
+164
| auto args_with_comma = [](const std::string& acc, const std::string& str) { | ||
| return acc.empty() ? str : acc + ", " + str; | ||
| }; | ||
| std::string result = std::accumulate(var.arguments.begin(), | ||
| var.arguments.end(), | ||
| std::string(), | ||
| args_with_comma); |
Member
There was a problem hiding this comment.
I'm a bit confused why this is done here and not in random_construct.cpp ? It's just nitpicking, but just wondering.
| } | ||
| } | ||
|
|
||
| std::vector<std::string> nrnlist_to_string(Item* begin, Item* qlist) { |
Member
There was a problem hiding this comment.
what's that for? I don't see its use here?
04ca7e3 to
be224cc
Compare
This comment has been minimized.
This comment has been minimized.
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
be224cc to
0d0789f
Compare
|
SonarCloud Quality Gate failed.
|
Collaborator
12 tasks
3 tasks
Member
Author
|
superseded by #2627 |
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
3 participants
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.
Summary of what has been done until now and how/why/where::
src/nrnoc/netstim_rng.modThis is the same as
netstim.modbut with the removal of all VERBATIM and save state-related features. This should replacenetstim.modonce all the implementation isfinished. I didn't modify
netstim.modbecause I didn't want to break existingtests. It uses new
RANDOMconstruct and methods likesample,init,setseq.src/nmodl/noccout.cppprint_functions_for_random_construct()emits all code related to helper+wrapper functionsthat we need in the generated code. As mentioned in the code comment, code like
_get_random_var_by_name()should be removed once we support proper syntax likenetstim.r.init_rng(id1,id2,id3).src/nmodl/parsact.cppHas changes to replace
sample, init, setseqfunctions with corresponding Random123 equivalent.src/nmodl/parse1.yppParser changes to parse RANDOM specification in NEURON block. This can be further
improved with error handling etc.
src/nmodl/random_construct.*Store random variables and supplementary methods to support code generation
src/oc/nrnran123.cppChanges to support distributions including NORMAL and UNIFORM.
test/coreneuron/test_rng.py: is an old test from hackathon to discuss the syntaxtest/nmodl/test_random.py: new test which tests the APIs and overall functionalityNext Steps
randomsemantic type in NEURONCurrently, we are registering a
randomsemantic but that is being treated the same asPOINTER.bbsavestateand transfer of this type to CoreNEURONnocmodlComments for Discussion
to
Improvements