Skip to content
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

NeuronTestHelper: CORENEURON option, reorganise. #1768

Merged
merged 2 commits into from
Apr 7, 2022

Conversation

olupton
Copy link
Collaborator

@olupton olupton commented Apr 6, 2022

  • Create nrnivmodl targets in nrn_add_test_group and add a CORENEURON option to that command. This means that we do not have to compile all MOD files for CoreNEURON if CoreNEURON is enabled in the build.
  • Drop MODFILE_PATTERNS, NRNIVMODL_ARGS and SUBMODULE arguments to nrn_add_test that were not being used.
  • The same de-duplication is done as before, so there is no change to the number of nrnivmodl invocations, but some targets no longer pass -coreneuron to nrnivmodl.

Motivated because I realised that the test framework was requiring that even MOD files for NEURON-only tests were compilable for CoreNEURON. This also speeds things up a little, as we skip some nrnivmodl-core invocations.

Also remove some test exclusions in testcorenrn because the referenced issue (BlueBrain/nmodl#311) was closed. Hopefully they will work now; https://bbpgitlab.epfl.ch/hpc/coreneuron/-/pipelines/46658 should confirm.

- Create nrnivmodl targets in nrn_add_test_group and add a CORENEURON
  option to that command. This means that we do not have to compile
  *all* MOD files for CoreNEURON if CoreNEURON is enabled in the build.
- Drop MODFILE_PATTERNS, NRNIVMODL_ARGS and SUBMODULE arguments to
  nrn_add_test that were not being used.
- The same de-duplication is done as before, so there is no change to
  the number of nrnivmodl invocations, but some targets no longer pass
  -coreneuron to nrnivmodl.
@codecov-commenter
Copy link

Codecov Report

Merging #1768 (dda1540) into master (bb838bc) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1768      +/-   ##
==========================================
+ Coverage   45.42%   45.43%   +0.01%     
==========================================
  Files         551      551              
  Lines      113149   113149              
==========================================
+ Hits        51393    51405      +12     
+ Misses      61756    61744      -12     
Impacted Files Coverage Δ
src/nrniv/nrncore_write/utils/nrncore_utils.cpp 62.50% <0.00%> (+3.57%) ⬆️
src/nrniv/prcellstate.cpp 93.49% <0.00%> (+6.50%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

src/nrnoc/hh.mod Outdated Show resolved Hide resolved
src/nrnoc/hh.mod Outdated Show resolved Hide resolved
BlueBrain/nmodl#311 has been closed, so hopefully these work now.
Copy link
Member

@pramodk pramodk left a comment

Choose a reason for hiding this comment

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

LGTM

@olupton olupton merged commit e403c27 into master Apr 7, 2022
@olupton olupton deleted the olupton/shuffle-tests branch April 7, 2022 06:27
@alexsavulescu alexsavulescu mentioned this pull request Jun 29, 2022
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants