-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make StochKv3.mod NEURON 9.0a compatible #35
Make StochKv3.mod NEURON 9.0a compatible #35
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks looks good to me. One point worthy mentioning is that in the requirements.txt
we had set the following restriction on the NEURON version for scientific reproducibility of the results.
NEURON>=7.8.0,<8.0.0
Maybe it can be added to the README.md that for the exact reproducibility of paper contents we recommend the aforementioned NEURON version range and the users using the versions outside that range may encounter variety in the results.
value = 0.5; | ||
// see BBPBGLIB-972 | ||
value = 0.0; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"The changes to mod files like StochKv3 are all technical. There is no change in intrinsic behaviour / results "
Just to remain "true" to the above quote: this change to value
variable is the one something that we were a bit unsure about. In the JIRA linked issue we explained why this was necessary and how this was the issue in load-balancing / ExperimentalMechComplex usage with NEURON.
We have ran simulations and results are same and now this change is in production. But if you really want to keep exact old code, you can set the value back to 0.5
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to add some more information if someone will revisit this in the future:
FUNCTION urand() {VERBATIM
double value;
if( usingR123 ) {
value = nrnran123_dblpick((nrnran123_State*)_p_rng);
} else if (_p_rng) {
#ifndef CORENEURON_BUILD
value = nrn_random_pick(RANDCAST _p_rng);
#endif
} else {
value = 0.5;
}
_lurand = value;
ENDVERBATIM
}
In the above urand()
function we are picking a random number. In all practical usage, we would have an RNG stream setup either using Random123 or MCellRan4. The last else
block returns a value 0.5 (or 0)
when a user hasn't set up the RNG variable i.e. StochKV is not properly instantiated. This won't be the case if someone is really using StochKV.
So I would say let's ignore my initial comment about this change. Using the new MOD file is perfectly OK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the detailed explanation @pramodk!
Update: after talking to @pramodk I tried running the tests with NEURON 8.0 with the USE_LEGACY flag BlueBrain/EModelRunner#81. The tests pass. We can remove the NEURON<8.0.0 restriction. I will try to do that in another PR. @darshanmandge have you tested this PR with NEURON 9.0? I am having a compilation error when I try it with NEURON 9.0. |
The StochKv3 file compiled successfully in a new python 3.11.6 virtual environment after installing neuron using |
I tried on linux desktop with python3.11.8 virtual environment and with
File "", line 684, in setitem I think we can wait to merge this till the final NEURON 9 release to solve these issues. |
This statement is not completely true. The optimisation tests do pass but the model-management tests do not pass with NEURON 8.2.4 |
@darshanmandge : Just to understand the above error: how can I reproduce the above? Certainly |
On my ubuntu desktop with 22.04.4 LTS, I create a python3.11.8 virtual environment using |
Could be this line: This indicates that Can you tell me which command you ran exactly that would reproduce this error? |
Sorry, I didn't specify the command. I compiled mod file using this command as the file was in the same folder: |
Ahh, so it should probably be this line then: If not, could you try finding the Python library file somehow (in the container, the path is |
I had minconda installed but I don't use it create the venv. Also, just FYI have have the default python 3.10 installed. |
The upper bound of NEURON version is relaxed from |
Thanks a lot @anilbey ! if it is fine, we can merge it. |
Alright. Let me just add notes to reproduce the precision differences in case it is of importance for the NEURON developers.
|
The PR makes StochKv3.mod with NEURON 9.0a. The file works with NEURON 8.2.2 and should also work with other version >8.* and the future NEURON 9 release as well. The file comes from BBP's internal repository.
According to @pramodk :
"The changes to mod files like StochKv3 are all technical. There is no change in intrinsic behaviour / results "