Cupy fixes

[bartvm]

A few fixes I had to make to make the Cupy branch run.

[mattjj]

Thanks for updating this! This looks like a great step forward. (EDIT: somehow lost this line the first time)

The commit history looks a bit messy. Maybe we can clean up the commit history by first rebasing HIPS:cupy onto HIPS:master, then rebasing any new commits here onto the result? (I can do the first step if that would help, then github can probably do the second.)

Also, I think the commit with message "Add vspace support for cupy" might be better as "Add flatten support for cupy". However, it's using the old flatten code. Jamie's new flatten code dispatches to the vspace and would probably be simpler to use here. (Also I had another implementation of flatten in cupy/autograd_util.py for use with cupy, but I think Jamie's new version should make flatten work for anything that has vspaces set up.)

[bartvm]

Sure, if you rebase cupy let me know and I'll rebase this branch.

Yes, it's using the flatten function from before 2 days ago, but the new one looks better. The current patch worked for CuPy arrays, but didn't work for e.g. lists of CuPy arrays (because the unflatten function would call np.split with CuPy arguments and crash) so optimizers wouldn't work.

[mattjj]

Just pushed the rebase. Can you resolve conflicts, or should I?

Good point re: unflatten, though can you point out which np.split call you mean? More generally, it's clear that SequenceVSpace uses numpy in its flatten function and so isn't general enough. I'm not sure how to fix that, but for now we could just have a separate flatten function (e.g. in autograd/cupy/autograd_util.py). What do you think?

[bartvm]

I rebased, commit history looks good now.

I generalized the vector spaces a bit so that they have a lib attribute with the correct numeric library. So instead np.flatten(x) you can write vspace(x).lib.flatten to make functions work on both NumPy and CuPy arrays. Note that this assumes that the values in lists or dictionaries are all of the same type (i.e. mixing CuPy and NumPy arrays doesn't work).

[bartvm]

PS The only thing that this change breaks is that the vector spaces of [] and () don't make sense anymore since we don't know what numerical library to use to initialize empty arrays for them.

[dougalm]

Hi Bart, thanks for working on this! Wrapping a good GPU library is probably the best thing we can do to get wider adoption of Autograd.

But I'm not totally convinced that this lib=numpy/lib=cupy approach is the right one. Here are some objections:

• It requires the wrapped numerical libraries to provide a bunch of functions: lib.random.randn, lib.random.randint, lib.zeros and lib.concatenate. This might be fine for numpy clones, but it isn't great if we start wrapping other libraries which might not have them, or might even have functions with the same names but different semantics.
• It adds code complexity to the Autograd core, including extra inner-loop operations like node creation.
• As you mentioned, it breaks empty containers and it doesn't handle containers with mixed numpy/cupy arrays.

An alternative approach is to have VSpace.flatten always return a numpy array. So CupyArrayVSpace.flatten would return a regular numpy array in CPU memory. This is going to be slow for large arrays, but it's fine for testing, which is the purpose of VSpace.flatten.

A drawback of this approach is that it gives numpy a special status. We like to hold on to the idea that Autograd should be backend-agnostic. But the truth is that numpy has always been a dependency and it's likely to remain that way, at least as a fallback library. It also helps to have a concrete array type for instantiating and evaluating tests, and numpy arrays are as good a choice as any. (There's an argument that Python's native array module would be a more natural choice but we've never gone down that road.)

Are there other objections to this approach I'm not thinking of?

Then there's the question of the user-exposed function util.flatten, which we currently implement using VSpace.flatten. It's a very useful function and it needs to be fast. Like the optimizers, it's a function that should probably be considered out of scope for Autograd. But it's very handy and we've had it for a long time so we'll keep it. We've recently gone forth and back and forth again on the question of whether util.flatten should have its own separate implementation. The issues you're raising here make me think we should go back to the original approach where util.flatten doesn't use VSpace.flatten. The two use-cases have such different demands that I don't mind some code duplication. Since it's a high-level function, we could even tolerate a flag that specifies which numerical library you want to use, defaulting to numpy. That would solve the mixed types and empty container cases.

[j-towns]

This is exciting stuff!

Having only looked over the pr fairly briefly, I think I agree with @dougalm's points...

An alternative approach is to have VSpace.flatten always return a numpy array.

I reckon this is the right approach, provided that the performance isn't ridiculously terrible — if, say it led to the tests taking 10x longer to run I think that would be unacceptable. I have no experience with GPU computing but I'm assuming the situation won't be that bad.

Then have either @dougalm's suggestion for the high level util.flatten behaving differently according to a flag or @mattjj's suggestion of a separate flatten function just for cupy arrays (which I see is already implemented).

I'm wondering to what extent it's possible to test this new functionality on Travis, which has no GPU support, or some other CI. There's an option to install cupy without cuda, however I've tried doing that and almost none of cupy's own tests run on my machine (which has no GPU). I was hoping there might be some simulated GPU mode, but this doesn't seem to be the case. Anyone got any thoughts on testing?

[bartvm]

In reply to some of @dougalm's points: The inability to mix NumPy and CuPy arrays is not because of any approach, it's inherent to the libraries themselves. You can't operate on both data types at the same time. Returning a NumPy array by default in places will often just crash. If you want to be backend agnostic, the correct approach to empty container spaces would probably be to special case them and return empty lists instead of empty NumPy arrays when calling flatten, ones, etc. on them. It's the only way of being agnostic. That the lib approach requires wrapped libraries to have the same API is what allows you to avoid code duplication. If you want to wrap a library that has a different API, you can write a super thin wrapper around it. Overall, I haven't really followed the back and forth between flatten, but from a software design perspective it seems like a bad idea to duplicate logic for different backends. It's a matter of time before the code will rot. The performance aspect seems easy enough to address. You can write a custom "lib" function that acts like "vspace(x).lib" with almost no overhead.

[dougalm]

Returning a NumPy array by default in places will often just crash.

My proposal was to have CupyArrayVSpace.flatten copy data from the GPU to the CPU, returning a regular numpy array using, say, cupy.asnumpy. I haven't played with cupy much. Is there a situation where this is impossible?

The performance aspect seems easy enough to address. You can write a custom
"lib" function that acts like "vspace(x).lib" with almost no overhead.

The performance issue I was talking about wasn't overhead cost, it was the cost of copying data from the GPU to the CPU which will always be expensive! But for testing I think it's fine (subject to @j-towns' caveat that if it slows down the tests 10X it might get annoying).