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

Add initial cupy tests #4214

Merged
merged 3 commits into from
Jul 13, 2020
Merged

Add initial cupy tests #4214

merged 3 commits into from
Jul 13, 2020

Conversation

jacobtomlinson
Copy link
Contributor

Added some initial unit tests for cupy. Mainly to create a place for cupy tests to go and to check some basic functionality.

I've created a fixture which constructs the dataset from the Toy weather data example and converts the underlying arrays to cupy. Then I've added a test which checks that after calling operations such as mean and groupby the resulting DataArray is still backed by a cupy array.

The main penalty with working on GPUs is accidentally shunting data back and forth between the GPU and system memory. Copying data over the PCI bus is slow compared to the rest of the work so should be avoided. So this first test is checking that we are leaving things on the GPU.

Because this data copying is so expensive cupy have intentionally broken the __array__ method and introduced a .get method instead. This means that users have to be explicit in converting back to numpy and copying back to the main memory. Therefore we will need to add some logic to xarray to use .get in appropriate situations such as plotting.

@TomNicholas TomNicholas added the topic-arrays related to flexible array support label Jul 10, 2020
@TomNicholas
Copy link
Member

Hi Jacob, would it make sense to add a new CI job that has cupy so that these tests actually get run automatically on future PRs? At the moment they're just going to get skipped, but if you're planning to work on this it might be a good idea to automate the cupy integration tests early on?

@jacobtomlinson
Copy link
Contributor Author

@TomNicholas I've been trying to find a way to do this. In order for the cupy tests to be run the CI machine will need an NVIDIA GPU. But setting up a GPU CI would be a burden on the xarray team both technically and financially. This is why I'm just skipping for now.

For projects over in RAPIDS we have have a private GPU CI. We aren't in a position to integrate this here but we may be able to run nightlies. I'll update when I have more info.

@jacobtomlinson
Copy link
Contributor Author

I think what we can offer at the moment is to run a nightly against the default branch on some GPU hardware. If the nightly fails we will open an issue and look into resolving it.

Not as neat as hooking into PRs like the rest of the CI though. Hopefully this will improve over time.

@dcherian
Copy link
Contributor

run a nightly against the default branch on some GPU hardware. If the nightly fails we will open an issue and look into resolving it.

This is great! Thanks @jacobtomlinson

@jacobtomlinson
Copy link
Contributor Author

Ok great! Once this is merged we will get that set up on our end.

Copy link
Contributor

@dcherian dcherian left a comment

Choose a reason for hiding this comment

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

LGTM

@dcherian
Copy link
Contributor

I'm merging so that @jacobtomlinson can keep moving forward.

It isn't breaking anything too :)

@dcherian dcherian merged commit 52043bc into pydata:master Jul 13, 2020
@jacobtomlinson
Copy link
Contributor Author

Many thanks! We will start running nightlies from today.

@jacobtomlinson
Copy link
Contributor Author

Nightlies are running. Sorry we can't do more than this right now.

image

@jacobtomlinson jacobtomlinson deleted the init-cupy branch July 13, 2020 16:32
dcherian added a commit to dcherian/xarray that referenced this pull request Aug 16, 2020
* master:
  Add initial cupy tests (pydata#4214)
  Add 0.16.0 release summary
  New whatsnew section
  Release v0.16.0
  Minor reorg of whatsnew for 0.16.0 (pydata#4216)
  fix sphinx warnings (pydata#4199)
  pin isort (pydata#4206)
  get the colorbar label via public methods (pydata#4201)
  Bump minimum versions for 0.16 release (pydata#4175)
  Allow passing axis kwargs to plot (pydata#4020)
  Fix to_unstacked_dataset for single dimension variables. (pydata#4094)
  Improve the speed of from_dataframe with a MultiIndex (by 40x!) (pydata#4184)
  More pint compatibility: silence UnitStrippedWarnings (pydata#4163)
  Fix typo (pydata#4192)
  use the latest image of RTD (pydata#4191)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-arrays related to flexible array support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants