Skip to content

Check consistency of units selection for NEURON and CoreNEURON.#793

Merged
nrnhines merged 10 commits into
masterfrom
nrnunit-coreneuron
Nov 5, 2020
Merged

Check consistency of units selection for NEURON and CoreNEURON.#793
nrnhines merged 10 commits into
masterfrom
nrnunit-coreneuron

Conversation

@nrnhines

@nrnhines nrnhines commented Oct 26, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Companion to BlueBrain/CoreNeuron#408 and BlueBrain/mod2c#53
Partially responds to #521

Adds a test asserting that nernst, ghk, and gasconstant calculations are the same between NEURON and CoreNEURON and mod2c.

Note that test/coreneuron/test_units.py for default -DCORENRN_ENABLE_LEGACY_UNITS=OFF prints

ctest -VV
...
12: R 8.3144626181532395 8.3144626181532395
12: erev 63.551503219374858 63.551503219374858
12: ghk -8141.3474521410844 -8141.3474521410844

But with -DCORENRN_ENABLE_LEGACY_UNITS=OFF

export NRNUNIT_USE_LEGACY=1
ctest -VV
...
12: R 8.3145000000000007 8.3145000000000007
12: erev 63.543396337823452 63.543396337823452
12: ghk -8142.315292190824 -8142.315292190824

@nrnhines nrnhines requested review from alkino and pramodk October 26, 2020 18:36
@nrnhines nrnhines mentioned this pull request Oct 27, 2020
Merged
pramodk
pramodk approved these changes Nov 4, 2020
View reviewed changes

@pramodk pramodk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from one minor comment, this looks ok to me.

Comment thread test/pynrn/test_units.py Outdated
Comment on lines +27 to +28
print("ut.ghk=%s h.ghk=%s" % (str(ut.ghk), str(ghk_std)))
print("ut.erev=%s h.nernst=%s" % (str(ut.erev), str(erev_std)))

@pramodk pramodk Nov 4, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be remove these extra print statements?

Comment thread test/pynrn/test_units.py Outdated
Comment on lines +35 to +36
print("ut.ghk=%s h.ghk=%s" % (str(ut.ghk), str(ghk_std)))
print("ut.erev=%s h.nernst=%s" % (str(ut.erev), str(erev_std)))

@pramodk pramodk Nov 4, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

Comment thread test/coreneuron/test_units.py Outdated
@nrnhines

nrnhines commented Nov 5, 2020

Copy link
Copy Markdown
Member Author

Note that the units test work for modern units. They also work for legacy units but will need some changes that were introduced in PR #803 to avoid

      Start  5: rxd_tests
 5/13 Test  #5: rxd_tests ........................***Failed    4.42 sec
      Start  6: rxd_mpi_tests
 6/13 Test  #6: rxd_mpi_tests ....................***Failed    6.57 sec

For this a suitable configuration involves -DNRN_ENABLE_RX3D=OFF -DNRN_DYNAMIC_UNITS_USE_LEGACY=ON
For example:

cmake .. -DCMAKE_INSTALL_PREFIX=install -DIV_DIR=$HOME/neuron/ivcmake/build/install -DPYTHON_EXECUTABLE=`which python3` -DNRN_ENABLE_PYTHON_DYNAMIC=ON -DNRN_PYTHON_DYNAMIC="python2.7;python3.7" -DCMAKE_BUILD_TYPE=Debug -DNRN_ENABLE_TESTS=ON -DNRN_ENABLE_RX3D=OFF -DNRN_ENABLE_CORENEURON=ON -DNRN_DYNAMIC_UNITS_USE_LEGACY=ON

@nrnhines nrnhines merged commit 94147e5 into master Nov 5, 2020
@nrnhines nrnhines deleted the nrnunit-coreneuron branch November 5, 2020 02:43
@alexsavulescu alexsavulescu mentioned this pull request Dec 8, 2020
Closed
@alexsavulescu alexsavulescu mentioned this pull request Apr 26, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pramodk pramodk pramodk approved these changes
@alkino alkino Awaiting requested review from alkino

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nrnhines @pramodk