Skip to content

Conversation

@Nic-Ma
Copy link
Contributor

@Nic-Ma Nic-Ma commented Jul 20, 2020

Fixes #785 .

Description

This PR adapted from PyTorch C++ extension example: https://pytorch.org/tutorials/advanced/cpp_extension.html

Quickly summarise the progress made so far by this PR:

as an initial step of testing/packaging/distributing cuda kernels in monai, it addresses

  1. unit testing and runtests.sh updates for c++/cuda extensions support
  2. python setup.py develop to compile the extensions (this is only a workaround as it ignores both pyproject.toml's build-system and pep517
  3. combining 1. and 2. in the automated CI pipelines
  4. the extensions are loosely coupled with monai, not compiling them only generates warnings and the rest of the package works normally

things that are not covered by this PR:

  • make/test/package/distribute prebuilt binaries within wheels with some targeted environments (cuda/cudnn/gcc... versions)
  • coding style enforcement and benchmarking for the cpp/cuda extensions

Status

ready

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh --codeformat --coverage.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Jul 20, 2020

I tried to compile this CUDA extension in MONAI docker directly, it completed successfully.
And tried to load and test:

>>> import torch
>>> import lltm_cuda
>>> lltm_cuda.forward
<built-in method forward of PyCapsule object at 0x7fc9061fc090>

Thanks.

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Jul 27, 2020

/black

@Nic-Ma Nic-Ma changed the title [WIP] 785 add test program for CUDA extension 785 add test program for CUDA extension Jul 28, 2020
@Nic-Ma Nic-Ma marked this pull request as ready for review July 28, 2020 03:51
@Nic-Ma Nic-Ma requested review from atbenmurray, ericspod and wyli July 28, 2020 03:51
@wyli
Copy link
Contributor

wyli commented Jul 28, 2020

/black

@wyli
Copy link
Contributor

wyli commented Jul 28, 2020

some mypy issue here?

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Jul 28, 2020

some mypy issue here?

Yes, now I updated according to the latest tutorial code.
Thanks.

@wyli
Copy link
Contributor

wyli commented Jul 28, 2020

I feel this feature is an important and unique one, shall we make it a top-level module, such as monai.ops.lltm_cuda ("ops" short for "operations"), and in the monai.networks.LLTM module, we optionally import monai.ops.lltm_cuda.

@wyli
Copy link
Contributor

wyli commented Jul 28, 2020

could you help remove this temporary file, I think it is included by mistake: https://github.com/Project-MONAI/MONAI/blob/master/tests/testing_data/._Task04_Hippocampus

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Jul 28, 2020

could you help remove this temporary file, I think it is included by mistake: https://github.com/Project-MONAI/MONAI/blob/master/tests/testing_data/._Task04_Hippocampus

Sure, will remove it in the next commit.
Thanks.

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Jul 28, 2020

/black

@Nic-Ma Nic-Ma changed the title 785 add test program for CUDA extension [WIP] 785 add test program for CUDA extension Jul 28, 2020
@wyli
Copy link
Contributor

wyli commented Jul 28, 2020

@Nic-Ma progress so far:

  1. in our case, pip will use pyproject.toml to configure the building process but our pyproject.toml doesn't look good, (missing a [build-system] section)

to resolve 1:
option a, remove pyproject.toml <- can't do it because psf/black needs it
option b, we can add a build-system section <- this causes further issues about the inconsistent torch version in build (always the latest) and run (we test different torch versions) the cpp extensions

looks like we need to address option b properly

additionally, there's a mypy issue...

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Jul 28, 2020

Hi @wyli ,

Thanks very much for your help!

@wyli wyli changed the title [WIP] 785 add test program for CUDA extension 785 add test program for CUDA extension Jul 30, 2020
@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Jul 30, 2020

Hi @wyli ,

Thanks very much for your help here, I think it looks good to me now.

@wyli
Copy link
Contributor

wyli commented Jul 31, 2020

this is ready @Nic-Ma (delayed because I wanted to add some pip install cache so that the CI tests are slightly more robust)

@Nic-Ma
Copy link
Contributor Author

Nic-Ma commented Jul 31, 2020

this is ready @Nic-Ma (delayed because I wanted to add some pip install cache so that the CI tests are slightly more robust)

OK, sounds good!
Thanks.

@Nic-Ma Nic-Ma merged commit e409445 into Project-MONAI:master Jul 31, 2020
@rijobro rijobro mentioned this pull request Nov 2, 2020
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.

Add sample CUDA program based on PyTorch C++ extension

3 participants