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

Conversation

@olupton
Copy link
Contributor

@olupton olupton commented Jul 20, 2021

Description
This modifies how we allocate state variables for Random123 in GPU-enabled builds. This should simplify things, and enable use-cases like in #528 where we need to manipulate this state on the device from CPU code in ways that the existing API did not allow.

Because the state is now accessible from both CPU and GPU code, this also closes #345.

How to test this?
Build and run with/without GPU support enabled.

Test System

  • OS: BB5
  • Compiler: NVHPC 21.2
  • Version: master
  • Backend: CPU/GPU

Use certain branches for the SimulationStack CI

CI_BRANCHES:NEURON_BRANCH=olupton/random123-unified-memory,

@olupton olupton force-pushed the olupton/random123-unified-memory branch from ce4bc03 to 8576f8d Compare July 21, 2021 07:23
Copy link
Collaborator

@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 force-pushed the olupton/random123-unified-memory branch from 5375bbd to f8cc054 Compare July 21, 2021 11:36
@codecov-commenter
Copy link

Codecov Report

Merging #595 (f8cc054) into master (9bd58f6) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #595      +/-   ##
==========================================
- Coverage   56.46%   56.45%   -0.02%     
==========================================
  Files          99      100       +1     
  Lines        8021     8030       +9     
==========================================
+ Hits         4529     4533       +4     
- Misses       3492     3497       +5     
Impacted Files Coverage Δ
coreneuron/utils/randoms/nrnran123.cpp 36.36% <ø> (-0.78%) ⬇️
coreneuron/utils/randoms/nrnran123.h 100.00% <100.00%> (ø)
coreneuron/network/multisend_setup.cpp 81.45% <0.00%> (+0.07%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9bd58f6...f8cc054. Read the comment docs.

@olupton olupton force-pushed the olupton/random123-unified-memory branch from f8cc054 to fdc8aaf Compare July 21, 2021 12:58
@olupton olupton marked this pull request as ready for review July 21, 2021 13:04
Copy link
Collaborator

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

Once channel-benchmark is validated to run with a single exe on CPU as well as GPU, this is good to go!

@pramodk
Copy link
Collaborator

pramodk commented Jul 23, 2021

Retest this please

@olupton
Copy link
Contributor Author

olupton commented Jul 23, 2021

It was tested locally that channel-benchmark produces consistent results with/without --gpu in a GPU-enabled build.

Note that a bug in the NVHPC compiler version 21.3 and 21.5 means version 21.2 (or older?) or 21.7 (or hopefully newer!) must be used.

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.

LGTM 👍

@pramodk
Copy link
Collaborator

pramodk commented Jul 23, 2021

Replayed rxd-mpi tests and they passed - https://bbpgitlab.epfl.ch/hpc/coreneuron/-/jobs/38730 (in previous CI there was timeout)

@pramodk pramodk merged commit 864b712 into master Jul 23, 2021
@pramodk pramodk deleted the olupton/random123-unified-memory branch July 23, 2021 18:24
pramodk pushed a commit to neuronsimulator/nrn that referenced this pull request Nov 2, 2022
* Unified Memory in Random123.
* Use OMP_Mutex instead of std::atomic, drop detail.
* Load gcc module for CUDA in Jenkins CI.

CoreNEURON Repo SHA: BlueBrain/CoreNeuron@864b712
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Random123 in VERBATIM : Allow GPU built executable to run on CPU

6 participants