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

missing API against TH #70

Open
30 of 35 tasks
soumith opened this issue Nov 6, 2014 · 52 comments
Open
30 of 35 tasks

missing API against TH #70

soumith opened this issue Nov 6, 2014 · 52 comments
Labels

Comments

@soumith
Copy link
Member

soumith commented Nov 6, 2014

The following math functions are missing in THC but present in TH:

  • numel
  • prod
  • zeros
  • ones
  • reshape
  • rand
  • randn
  • round
  • atan2
  • std (not stdall, this is per-dimension)
  • var (not varall, this is per-dimension)
  • cumsum
  • cumprod
  • maskedFill
  • maskedSelect
  • sort
  • maskedCopy (waiting for merge, Streams for BLAS. maskedCopy. generic reduce kernels. bugfix. #167)
  • multinomial (implemented, waiting for PR)
  • cross
  • logicalall
  • logicalany
  • tril
  • triu
  • trace
  • diag
  • nonzero
  • range
  • cat
  • linspace
  • logspace
  • eye
  • randperm
  • histc
  • conv2 (these are torch operators, not to be confused with nn Convolutions which are already implemented)
  • conv3

When these are implemented, cwrap entries can be added that would make cutorch completely API compatible with torch

@soumith soumith added the bug label Nov 6, 2014
This was referenced Nov 7, 2014
@clementfarabet
Copy link
Member

Hey @soumith , so running cutorch.test() currently reports errors on cumsum and cumprod (missing). That's on purpose?

@soumith
Copy link
Member Author

soumith commented Nov 8, 2014

Yes they are yet to be implemented on cutorch. I'll get to that next week. They can be done with a thrust:: scan

@clementfarabet
Copy link
Member

Ok cool.

On Sat, Nov 8, 2014 at 12:27 PM, Soumith Chintala [email protected]
wrote:

Yes they are yet to be implemented on cutorch. I'll get to that next week.
They can be done with a thrust:: scan


Reply to this email directly or view it on GitHub
#70 (comment).

@dominikgrewe
Copy link
Member

Does anyone have an implementation of std and var yet? Otherwise I'll have a go next week.
Afaict, we can't use THCudaTensor_reduceDim so we'll need something custom (but with a similar structure as reduceDim).

@soumith
Copy link
Member Author

soumith commented Jan 3, 2015

no, do not have it. this has been long overdue, but we should co-ordinate these, let me email you guys.

@dominikgrewe
Copy link
Member

Any progress on cumsum and cumprod?

@soumith
Copy link
Member Author

soumith commented Jan 20, 2015

have not started them yet!

@dominikgrewe
Copy link
Member

Okay, I might have a go soon then, because we'd like to use it. Just waiting for the THCState PR to be merged.

@soumith
Copy link
Member Author

soumith commented Jan 21, 2015

I will merge the THCState PR on Friday. All the patches have been prepared except for fbcunn, working on that as well.

@dominikgrewe
Copy link
Member

@soumith For reductions along a single dimension, we still have the restriction that the tensor must not have more than 4 dimensions. Did you say you're working on a fix for that? If so, what's the progress on it?
If you're not working on it, we could easily change the code to what we do for std and var, where there's no restriction on dimensionality at all.

@soumith
Copy link
Member Author

soumith commented Feb 19, 2015

@wickedfoo already has PR for that internally. Its all implemented. I'm on
vacay till 26th, so I will sync those changes at the end of this month.
On Feb 19, 2015 4:34 PM, "Dominik Grewe" [email protected] wrote:

@soumith https://github.com/soumith For reductions along a single
dimension, we still have the restriction that the tensor must not have more
than 4 dimensions. Did you say you're working on a fix for that? If so,
what's the progress on it?
If you're not working on it, we could easily change the code to what we do
for std and var, where there's no restriction on dimensionality at all.

Reply to this email directly or view it on GitHub
#70 (comment).

@soumith
Copy link
Member Author

soumith commented Feb 19, 2015

His PR is for apply (and apply2) along an arbitrary dimension. Not reductions. It generalizes the copy kernels and changes all the tensor math to use these apply kernels where appropriate instead of make contiguous + thrust

@soumith
Copy link
Member Author

soumith commented Feb 19, 2015

That's the status on that. He did not work yet on arbitrary reductions. If you want to tackle that, go for it.

@dominikgrewe
Copy link
Member

Thanks for the update. I'll have a go at the reductions kernel then.
Looking forward to the apply kernels!

@wickedfoo
Copy link
Contributor

I'm on vacation too until next week but I don't think the generic apply
stuff got pushed yet, I think only the old version of the copy kernel. I
reimplemented all cutorch math (pointwise operators) in terms of it, so no
newContiguous calls are needed. I don't yet support reductions but it
shouldn't be too hard to add that in.

On Thursday, February 19, 2015, Dominik Grewe [email protected]
wrote:

Thanks for the update. I'll have a go at the reductions kernel then.
Looking forward to the apply kernels!


Reply to this email directly or view it on GitHub
#70 (comment).

@dominikgrewe
Copy link
Member

If I remember correctly, you guys said you'd look into maskedFill etc, right? Any progress on that?

@wickedfoo
Copy link
Contributor

for maskedFill etc. do you want the mask to be a float vector (because that's the only thing we have in cutorch at present), or do you want it to be 4 bytes packed into a float?

@dominikgrewe
Copy link
Member

I guess float vectors make the most sense, because that's what logical functions (gt, ge etc) return.

@wickedfoo
Copy link
Contributor

I have maskedFill/Copy/Select done, and sort() I have power-of-2 sizes at present (but on input with an arbitrary number of dimensions), so still working on that. maskedFill, maskedCopy and sort avoid newContiguous on the input, but maskedSelect I chickened out and just used two passes and temporary space with a Thrust prefix scan.

Re: "For reductions along a single dimension, we still have the restriction that the tensor must not have more than 4 dimensions. Did you say you're working on a fix for that? If so, what's the progress on it?" I have this fixed as well, took the copy kernel code and made a reduction kernel out of it, so no calls to newContiguous/copies etc. needed. Not a global reduction kernel (like a norm that reduces down to one point, but reduces along a dimension. sort() exploits similar code. I want to do the same shared memory optimization (so I can use coalesced reads) that you did if the reduction dimension is innermost/most contiguous though.

@dominikgrewe
Copy link
Member

Cool, looking forward to that. Yes, using the shared memory approach for reductions along contiguous dimensions is vital.

@dominikgrewe
Copy link
Member

When do you think you'll have a PR for maskedFill etc?

@soumith
Copy link
Member Author

soumith commented Mar 5, 2015

it's in review, and jeff is still working on revamping our code-base to the state argument based change.
Hopefully sometime next week.

And the cutorch TensorMath changes that remove most of the sync points on non-contiguous cases will also land at the same time.

@jaderberg
Copy link

Any progress on the masked* functions?

@soumith
Copy link
Member Author

soumith commented Mar 12, 2015

Theyre implemented. We are working on refactoring our code and syncing.with master, we will try to merge them this week.

@soumith
Copy link
Member Author

soumith commented Mar 13, 2015

An update:
Jeff has powered through our internal refactor, getting us back to parity with oss cutorch.
There are three PRs coming up.

  • masked* functions
  • sort
  • all of the math (where applicable) revamped to use our own pointwise and reduce kernels, so that non-contiguous tensors are no longer sync points.

It will either be EOD today or most likely Monday/Tuesday.

@soumith
Copy link
Member Author

soumith commented Mar 13, 2015

Looks like what's left is the small fish.

  • torch.diag, torch.eye, torch.trace can be handled with the same generic diagonal-apply kernel.
  • torch.randperm, linspace, logspace, range is a thrust::scan
  • logicalall, logicalany also apply kernel!?
  • tril and triu might be tricky

@dominikgrewe
Copy link
Member

There are a number of linear algebra functions missing: symeig, eig, inverse etc. In Torch they seem to be implemented by wrapping Lapack. Could we do something similar for cutorch? There's MAGMA and CULA; does anyone have experience with these libraries?

@soumith
Copy link
Member Author

soumith commented Apr 1, 2015

MAGMA looks best, we built MAGMA internally and it looks reasonably good.

@soumith
Copy link
Member Author

soumith commented Apr 1, 2015

Also, on the CuDNN note, we can configure a header (like THGeneral.h.in ) if we find cudnn. Caffe has the cmake macros needed for finding cudnn already written: https://github.com/BVLC/caffe/blob/master/cmake/Cuda.cmake

@dpfau
Copy link

dpfau commented May 24, 2015

Hi guys. Noticed this thread when dealing with a script that needs diag, svd and eig on CudaTensors. I implemented diag myself in Lua using storage() and set(), but svd and eig are beyond my ken. What's the plan for that?

@soumith
Copy link
Member Author

soumith commented May 25, 2015

One of my colleagues @SamGross is working on it by interfacing the magma cuda library. It'll happen over the next month or so when he finishes it up and sends a PR.

@wickedfoo
Copy link
Contributor

This is not on this list, but I'm in the process of implementing THCudaTensor_multinomial as well.

@nicholas-leonard
Copy link
Member

@wickedfoo Awesome. Would love to see multinomial in cuda.

@hughperkins
Copy link
Contributor

Just to confirm, scatter/gather arent implemented in cutorch, right?

@dominikgrewe
Copy link
Member

That's right. I meant to do it, but haven't had the time yet, sorry.

@hughperkins
Copy link
Contributor

For gather, which I suddenly realize could be useful for implementing ClassNLLCriterion.forward, without needing a custom kernel, I guess?, I suppose a simple naive first-cut could be:

  • use the isContiguous (toContiguous? asContiguous?) method to convert the tensor to contiguous format
  • simply assign one thread to each output location, and I dont think we need any local memory or anything right? So, just throw everything into warp-size blocks and it's basically done?

Does that sound about right? Anything else I should bear in mind if I write a naive gather along these lines? (I'll be targeting cltorch I confess, but it's quite easy to convert cutorch<->cltorch kernels I think?)

(Edit: what do you think is the most similar existing class/kernel to base this off? and/or thoughts on where to put this, ie filename(s)?)

@hughperkins
Copy link
Contributor

One of my colleagues @SamGross is working on it by interfacing the magma cuda library. It'll happen over the next month or so when he finishes it up and sends a PR.

Magma looks cool. Has opencl version too it seems :-)

@hughperkins
Copy link
Contributor

Shoe-horned the lua wrapper into TensorMath.lua: hughperkins/cltorch@0e469f4

@hughperkins
Copy link
Contributor

@abyravan
Copy link

Is there any update on adding these functions? I'm in need of the cross product on cudatensors.

I can code it up if someone can point me towards the things that need to be done. All the layers I've written so far are on the lua side and I'm not sure how to make the connection between cuda and lua. Thanks!

@soumith
Copy link
Member Author

soumith commented Feb 28, 2016

@abyravan i could get to cross next week.

@abyravan
Copy link

That would be great. Thanks a lot! Is there any sort of a tutorial or howto on adding new functionality for tensors? Would be useful to have :)

@soumith
Copy link
Member Author

soumith commented Feb 28, 2016

You can look at existing PRs that are cross-linked in this thread.
Like:
#120
#96
#75

@soumith
Copy link
Member Author

soumith commented Feb 28, 2016

nonzero is being implemented by FB, should be out in a few days.

archenroot pushed a commit to archenroot/gentoo-overlay that referenced this issue Dec 4, 2016
** NOTE on API changes and versioning **

Cutorch provides a CUDA backend for torch7.

Cutorch provides the following:

a new tensor type: torch.CudaTensor that acts like torch.FloatTensor, but all it's operations are on the GPU. Most of the tensor operations are supported by cutorch. There are a few missing ones, which are being implemented. The missing list can be found here: torch/cutorch#70
several other GPU tensor types, with limited functionality. Currently limited to copying/conversion, and several indexing and shaping operations.
cutorch.* - Functions to set/get GPU, get device properties, memory usage, set/get low-level streams, set/get random number generator's seed, synchronization etc. They are described in more detail belo
@pvtokmakov
Copy link

Hi guys,

any hope on implementing the conv2 (or xcorr2 for that matter)?

@ethanluoyc
Copy link
Contributor

Just adding a note that eye has been implemented here pytorch/pytorch#2395

@sjain-stanford
Copy link

Thanks for supporting many of the math functions in THC. I'm now waiting for histc!

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

No branches or pull requests