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

Reorganise "global" variables to not be global #904

Merged
merged 63 commits into from
Aug 28, 2022

Conversation

olupton
Copy link
Contributor

@olupton olupton commented Aug 5, 2022

See BlueBrain/CoreNeuron#795 and BlueBrain/mod2c#78.

TODO:

CI_BRANCHES:CORENEURON_BRANCH=pramodk/exclude-global-vars,NEURON_BRANCH=olupton/coreneuron-gpu-dynamic-loading

@bbpbuildbot

This comment was marked as outdated.

@bbpbuildbot

This comment was marked as outdated.

.gitmodules Outdated Show resolved Hide resolved
@bbpbuildbot

This comment was marked as outdated.

@bbpbuildbot

This comment was marked as outdated.

@bbpbuildbot

This comment was marked as outdated.

@bbpbuildbot

This comment was marked as outdated.

@bbpbuildbot

This comment was marked as outdated.

@bbpbuildbot

This comment was marked as outdated.

@bbpbuildbot

This comment was marked as outdated.

@bbpbuildbot

This comment was marked as outdated.

@bbpbuildbot
Copy link
Collaborator

Logfiles from GitLab pipeline #70967 (:no_entry:) have been uploaded here!

Status and direct links:

@olupton olupton requested a review from pramodk August 24, 2022 12:15
@bbpbuildbot
Copy link
Collaborator

Logfiles from GitLab pipeline #71003 (:white_check_mark:) have been uploaded here!

Status and direct links:

@bbpbuildbot
Copy link
Collaborator

Logfiles from GitLab pipeline #71039 (:white_check_mark:) have been uploaded here!

Status and direct links:

Copy link
Contributor

@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.

see if there is anything relevant from the minor comments otherwise Good to Go!

src/visitors/implicit_argument_visitor.cpp Show resolved Hide resolved
src/visitors/implicit_argument_visitor.cpp Show resolved Hide resolved
src/codegen/codegen_ispc_visitor.cpp Show resolved Hide resolved
src/codegen/codegen_helper_visitor.cpp Show resolved Hide resolved
@bbpbuildbot
Copy link
Collaborator

Logfiles from GitLab pipeline #71265 (:no_entry:) have been uploaded here!

Status and direct links:

@bbpbuildbot
Copy link
Collaborator

Logfiles from GitLab pipeline #71277 (:white_check_mark:) have been uploaded here!

Status and direct links:

@olupton olupton force-pushed the olupton/global-variables branch 2 times, most recently from c7891c5 to 4eaad0b Compare August 25, 2022 15:19
@bbpbuildbot
Copy link
Collaborator

Logfiles from GitLab pipeline #71295 (:white_check_mark:) have been uploaded here!

Status and direct links:

Copy link
Contributor

@iomaganaris iomaganaris left a comment

Choose a reason for hiding this comment

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

Just few questions/comments while skimming through the changes

src/codegen/codegen_c_visitor.cpp Show resolved Hide resolved
src/codegen/codegen_c_visitor.cpp Show resolved Hide resolved
@pramodk pramodk merged commit 4f45a1c into master Aug 28, 2022
@pramodk pramodk deleted the olupton/global-variables branch August 28, 2022 11:51
pramodk added a commit to BlueBrain/CoreNeuron that referenced this pull request Aug 28, 2022
…#795)

* coreneuron and mechanism library can be built as shared and it
  enables launching coreneuron on GPU via python
* update MOD2C and NMODL fixes to handle GLOBAL variables
      See BlueBrain/mod2c/pull/78
      See BlueBrain/nmodl/pull/904
* removed acc/openmp global annotations for celsius, pi and secondorder
  and they don't need to be copied on GPU
* Pass Memb_list* as an argument for all common prototypes in order
   to support global variables via argument
* free ml->instance if not empty
* add link to libscopmath in neuron as well
* nrn_ghk is now declared inline.
* homegrown present table to avoid dynamic loading + acc_deviceptr limitations
* use -gpu=nordc and make #pragma acc routine seq functions inline
* drop -lscopmath as its folded in elsewhere
* random123 header reorganisation
* try and cleanup CLI11 handling.
* try and consolidate build logic
* some CORENEURON_ -> CORENRN_ for consistency.
* export OpenACC flags to NEURON separately as well as part
     of the whole ... -lcoreneuron ... link line.
* libcoreneuron.so -> libcorenrnmech.so, try and fix static builds
* do not enable OpenMP in shared/OpenACC builds.
* add rpaths inside nrnivmodl-core.
* accept a private destructor function pointer from generated mechanisms
* drop ${TEST_EXEC_PREFIX} that was causing simple tests to be executed on many ranks.
* CORENEURON_GPU_DEBUG: add environment variable that enables cnrn_target_* debug messages.

fixes #141

Co-authored-by: Olli Lupton <oliver.lupton@epfl.ch>
pramodk added a commit to neuronsimulator/nrn that referenced this pull request Nov 2, 2022
…BlueBrain/CoreNeuron#795)

* coreneuron and mechanism library can be built as shared and it
  enables launching coreneuron on GPU via python
* update MOD2C and NMODL fixes to handle GLOBAL variables
      See BlueBrain/mod2c/pull/78
      See BlueBrain/nmodl/pull/904
* removed acc/openmp global annotations for celsius, pi and secondorder
  and they don't need to be copied on GPU
* Pass Memb_list* as an argument for all common prototypes in order
   to support global variables via argument
* free ml->instance if not empty
* add link to libscopmath in neuron as well
* nrn_ghk is now declared inline.
* homegrown present table to avoid dynamic loading + acc_deviceptr limitations
* use -gpu=nordc and make #pragma acc routine seq functions inline
* drop -lscopmath as its folded in elsewhere
* random123 header reorganisation
* try and cleanup CLI11 handling.
* try and consolidate build logic
* some CORENEURON_ -> CORENRN_ for consistency.
* export OpenACC flags to NEURON separately as well as part
     of the whole ... -lcoreneuron ... link line.
* libcoreneuron.so -> libcorenrnmech.so, try and fix static builds
* do not enable OpenMP in shared/OpenACC builds.
* add rpaths inside nrnivmodl-core.
* accept a private destructor function pointer from generated mechanisms
* drop ${TEST_EXEC_PREFIX} that was causing simple tests to be executed on many ranks.
* CORENEURON_GPU_DEBUG: add environment variable that enables cnrn_target_* debug messages.

fixes BlueBrain/CoreNeuron#141

Co-authored-by: Olli Lupton <oliver.lupton@epfl.ch>

CoreNEURON Repo SHA: BlueBrain/CoreNeuron@12272f8
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.

5 participants