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

Nipype multiproc plugin modification to use GPU(s) as resources. #2298

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

schahid
Copy link

@schahid schahid commented Nov 22, 2017

Fixes # .

Changes proposed in this pull request

  • Workflow nodes that run on GPUs can be scheduled similar to that of CPU resources. The plugin args will now require additionally "n_gpus" for total no. of GPUs to use and "ngpuproc" for no. of threads to start on a single GPU.

self._cmd = command or getattr(self, '_cmd', None)

if self._cmd is None:
if not hasattr(self, '_cmd'):
Copy link
Member

Choose a reason for hiding this comment

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

this bit should be reverted as the above command is equivalent.

self.n_gpu_proc = self.plugin_args.get('ngpuproc', 1)

# Check plugin args
if self.plugin_args:
Copy link
Member

Choose a reason for hiding this comment

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

i think this if self.plugin_args can be eliminated.


#form a GPU queue first
gpus=[]
try:
Copy link
Member

Choose a reason for hiding this comment

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

this try catch is similar to gpu_count below. perhaps can be refactored to a common function.

@satra
Copy link
Member

satra commented Jan 20, 2018

it would be good to merge this with master and resolve the conflicts.

@satra
Copy link
Member

satra commented Feb 15, 2018

@schahid - is this something you can take care of? or should we take it over?

@schahid
Copy link
Author

schahid commented Feb 21, 2018

@satra - sorry for the delay in replying...as i am on holidays...yes sure you can take it over. thanks a lot.

@satra
Copy link
Member

satra commented Mar 7, 2018

just putting a note here that with the reorganization of this module in release 1.0, this will require some significant merge effort.

@effigies
Copy link
Member

effigies commented Mar 7, 2018

Foolishly enough, I took a shot at that some time last week.

master...effigies:enh/gpu_management

Since it's @schahid's master branch, I don't want to push to it directly. If anybody wants to take ownership of this PR, they can push that branch to their own repo, and open a new PR. (Or @schahid can git merge my branch into their master and resume work on it, .)

@satra
Copy link
Member

satra commented Mar 7, 2018

@effigies :)

@schahid - would it be possible for you to take it on from @effigies changes?

@schahid
Copy link
Author

schahid commented Mar 8, 2018

Hi @satra and @effigies
I have merged your branch into my master branch. Please let me know what else I should do?
Thanks.

@codecov-io
Copy link

codecov-io commented May 25, 2018

Codecov Report

Merging #2298 into master will decrease coverage by 0.23%.
The diff coverage is 47.02%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2298      +/-   ##
==========================================
- Coverage   66.66%   66.42%   -0.24%     
==========================================
  Files         328      339      +11     
  Lines       42529    47398    +4869     
  Branches     5278     5317      +39     
==========================================
+ Hits        28350    31485    +3135     
- Misses      13500    15171    +1671     
- Partials      679      742      +63
Flag Coverage Δ
#smoketests 50.66% <44.86%> (-0.13%) ⬇️
#unittests 63.96% <40.54%> (-0.02%) ⬇️
Impacted Files Coverage Δ
nipype/pipeline/plugins/multiproc.py 60.41% <47.02%> (-19.35%) ⬇️
nipype/interfaces/nilearn.py 40% <0%> (-56.83%) ⬇️
nipype/interfaces/nitime/analysis.py 43.15% <0%> (-36.85%) ⬇️
nipype/utils/nipype_cmd.py 41.66% <0%> (-35%) ⬇️
nipype/testing/fixtures.py 77.33% <0%> (-21.34%) ⬇️
nipype/pipeline/plugins/tools.py 70.32% <0%> (-16.82%) ⬇️
nipype/algorithms/confounds.py 51.71% <0%> (-15.6%) ⬇️
nipype/interfaces/freesurfer/base.py 60.16% <0%> (-13.01%) ⬇️
nipype/interfaces/ants/registration.py 67.04% <0%> (-6.04%) ⬇️
nipype/workflows/smri/freesurfer/autorecon2.py 65.9% <0%> (-5.37%) ⬇️
... and 96 more

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 88dbce1...1e1adc8. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants