Skip to content
This repository was archived by the owner on Mar 20, 2023. It is now read-only.

Conversation

@pramodk
Copy link
Collaborator

@pramodk pramodk commented Apr 3, 2022

Description

Summary

  • mod2c now generates code without need of global variables
  • coreneuron and mechanism library can be built as shared and it
    enables launching coreneuron on GPU via pyton
  • scopmath library can be also shared
  • removed acc/openmp global annotations for celsius, pi and secondorder
    and they don't need to be copied on GPU

TODOs

Closes #141.
Closes #599.

How to test this?

Build neuron master with this PR branch of coreneuron including updated mod2c submodule.

module load unstable gcc nvhpc cuda hpe-mpi cmake  python-dev

cmake .. -DCMAKE_INSTALL_PREFIX=`pwd`/install -DCORENRN_ENABLE_GPU=ON -DCORENRN_ENABLE_NMODL=OFF -DCORENRN_ENABLE_MPI=ON  -DNRN_ENABLE_CORENEURON=ON -DNRN_ENABLE_INTERVIEWS=OFF -DNRN_ENABLE_TESTS=OFF
make -j12
make install

No compile ringtest with nrnivmodl -coreneuron and launch ringtest on GPU using python as well as special. See #141 (comment)

Test System

  • Compiler: NVHPC & CUDA on BB5
  • Version: this PR and neuron@master
  • Backend: GPU

CI_BRANCHES:NEURON_BRANCH=olupton/coreneuron-gpu-dynamic-loading,SPACK_BRANCH=olupton/coreneuron-gpu-dynamic

@pramodk pramodk added the gpu label Apr 3, 2022
@pramodk pramodk marked this pull request as draft April 3, 2022 22:45
@bbpbuildbot

This comment was marked as outdated.

@pramodk
Copy link
Collaborator Author

pramodk commented Apr 4, 2022

@olupton : to reproduce the linking issue:

git pull
git checkout pramodk/exclude-global-vars
git submodule update -f --init --recursive

temporarily update hh.mod with call to random123:

+++ b/coreneuron/mechanism/mech/modfile/hh.mod
@@ -112,6 +112,9 @@ UNITSOFF
        sum = alpha + beta
         ntau = 1/(q10*sum)
         ninf = alpha/sum
+        VERBATIM
+            double xxx = nrnran123_dblpick(nullptr);
+        ENDVERBATIM
 }

and build:

mkdir build_gpu_pr &&  cd build_gpu_pr
module load unstable gcc nvhpc cuda hpe-mpi cmake  python-dev
cmake .. -DCMAKE_INSTALL_PREFIX=`pwd`/install -DCORENRN_ENABLE_GPU=ON -DCORENRN_ENABLE_MPI=ON
make -j8

and this should give:

[ 81%] Running nrnivmodl-core with halfgap.mod
[INFO] Running: make -j1 -f /gpfs/bbp.cscs.ch/home/kumbhar/workarena/systems/bbpv/repos/bbp/coreneuron/build_gpu_pr/share/coreneuron/nrnivmodl_core_makefile ROOT=/gpfs/bbp.cscs.ch/home/kumbhar/workarena/systems/bbpv/repos/bbp/coreneuron/build_gpu_pr MOD2CPP_BINARY=/gpfs/bbp.cscs.ch/home/kumbhar/workarena/systems/bbpv/repos/bbp/coreneuron/build_gpu_pr/bin/mod2c_core MODS_PATH=x86_64/corenrn/mod2c BUILD_TYPE=SHARED NRN_PRCELLSTATE=0
Default NMODL flags:
NVC++-W-1057-Static variables are not supported in acc routine - _ZN49_INTERNAL_27_x86_64_corenrn_mod2c_hh_cpp_4898369610coreneuron21_global_variables_ptrE (x86_64/corenrn/mod2c/hh.cpp: 364)
NVC++-W-1057-Static variables are not supported in acc routine - _ZN49_INTERNAL_27_x86_64_corenrn_mod2c_hh_cpp_4898369610coreneuron21_global_variables_ptrE (x86_64/corenrn/mod2c/hh.cpp: 374)
NVC++-W-1057-Static variables are not supported in acc routine - _ZN49_INTERNAL_27_x86_64_corenrn_mod2c_hh_cpp_4898369610coreneuron21_global_variables_ptrE (x86_64/corenrn/mod2c/hh.cpp: 440)
ptxas fatal   : Unresolved extern function '_ZN10coreneuron17nrnran123_dblpickEPNS_15nrnran123_StateE'
NVC++-F-0155-Compiler failed to translate accelerator region (see -Minfo messages): Device compiler exited with error status code (x86_64/corenrn/mod2c/hh.cpp: 444)
NVC++/x86-64 Linux 22.2-0: compilation aborted
make[3]: *** [x86_64/corenrn/build/hh.o] Error 2
make[2]: *** [bin/x86_64/special-core] Error 2
make[1]: *** [coreneuron/CMakeFiles/nrniv-core.dir/all] Error 2

@olupton
Copy link
Contributor

olupton commented Apr 12, 2022

@bbpbuildbot

This comment was marked as outdated.

@pramodk pramodk force-pushed the pramodk/exclude-global-vars branch from b5083fd to 89baf7b Compare April 21, 2022 12:39
@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.

@olupton olupton force-pushed the pramodk/exclude-global-vars branch from 9b8fe22 to df32d6f Compare April 26, 2022 07:50
@bbpbuildbot

This comment was marked as outdated.

@olupton
Copy link
Contributor

olupton commented Apr 26, 2022

I think the CI failures are mainly because of build system issues, and using (apparently) not widely supported arguments to ar.

@bbpbuildbot

This comment was marked as outdated.

Copy link
Collaborator Author

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

I went through the changes quickly and I added my quick comments here. The only thing that stands out is random123 but that could be rediscussed once we have initial tests are working.

@olupton olupton force-pushed the pramodk/exclude-global-vars branch from 9ebd22d to 3989293 Compare April 28, 2022 08:27
@bbpbuildbot

This comment was marked as outdated.

@bbpbuildbot

This comment was marked as outdated.

@bbpbuildbot

This comment was marked as outdated.

@olupton olupton mentioned this pull request Jul 12, 2022
@olupton olupton force-pushed the pramodk/exclude-global-vars branch from e9f8930 to 4af041d Compare July 12, 2022 09:51
@bbpbuildbot

This comment was marked as outdated.

@olupton olupton force-pushed the pramodk/exclude-global-vars branch from 4af041d to 246918e Compare July 12, 2022 12:24
@bbpbuildbot

This comment was marked as outdated.

@bbpbuildbot

This comment was marked as outdated.

@olupton
Copy link
Contributor

olupton commented Jul 18, 2022

neuronsimulator/nrn#1922 is needed to make some tests (e.g. pynrn::basic_tests) pass on systems (e.g. BB5) where Numpy is built with Intel MKL.

@olupton olupton closed this Aug 25, 2022
@olupton olupton reopened this Aug 25, 2022
@bbpbuildbot
Copy link
Collaborator

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

Status and direct links:

@bbpbuildbot
Copy link
Collaborator

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

Status and direct links:

Copy link
Collaborator Author

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

Don't have anything specific to add here except that rpath question.

@bbpbuildbot
Copy link
Collaborator

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

Status and direct links:

@olupton olupton closed this Aug 26, 2022
@olupton olupton reopened this Aug 26, 2022
@bbpbuildbot
Copy link
Collaborator

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

Status and direct links:

@olupton olupton force-pushed the pramodk/exclude-global-vars branch from 4e0e386 to 0e24755 Compare August 26, 2022 13:01
@bbpbuildbot
Copy link
Collaborator

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

Status and direct links:

@bbpbuildbot
Copy link
Collaborator

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

Status and direct links:

@pramodk pramodk closed this Aug 28, 2022
@pramodk pramodk reopened this Aug 28, 2022
@bbpbuildbot
Copy link
Collaborator

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

Status and direct links:

Copy link
Collaborator Author

@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 - as this is discussed quite a bit and tested by Olli, I will merge this! 🚀

@codecov-commenter
Copy link

Codecov Report

Merging #795 (567bd21) into master (d6507e2) will decrease coverage by 0.75%.
The diff coverage is 24.68%.

@@            Coverage Diff             @@
##           master     #795      +/-   ##
==========================================
- Coverage   58.49%   57.74%   -0.76%     
==========================================
  Files         102      103       +1     
  Lines        9412     9459      +47     
==========================================
- Hits         5506     5462      -44     
- Misses       3906     3997      +91     
Impacted Files Coverage Δ
coreneuron/apps/corenrn_parameters.hpp 100.00% <ø> (ø)
coreneuron/apps/main1.cpp 45.28% <0.00%> (-0.87%) ⬇️
coreneuron/io/core2nrn_data_return.cpp 1.13% <0.00%> (ø)
coreneuron/io/nrn2core_data_init.cpp 0.00% <0.00%> (ø)
coreneuron/io/nrn_checkpoint.cpp 4.35% <ø> (ø)
coreneuron/mechanism/capac.cpp 61.81% <ø> (ø)
coreneuron/mechanism/eion.cpp 68.26% <0.00%> (+12.36%) ⬆️
coreneuron/mechanism/mechanism.hpp 20.00% <ø> (ø)
coreneuron/mechanism/membfunc.hpp 0.00% <0.00%> (ø)
coreneuron/mechanism/patternstim.cpp 0.00% <0.00%> (ø)
... and 25 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@pramodk pramodk merged commit 12272f8 into master Aug 28, 2022
@pramodk pramodk deleted the pramodk/exclude-global-vars branch August 28, 2022 16:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GPU-enabled builds should not call GPU APIs without --gpu Support for SHARED build with PGI OpenACC build

5 participants