warp-ctc icon indicating copy to clipboard operation
warp-ctc copied to clipboard

Fix build against the latest pytorch

Open akhti opened this issue 6 years ago • 3 comments

Cuda interface was changed in https://github.com/pytorch/pytorch/commit/1003ccfa15e944251a65ba2289f25e8f1ed14a46

akhti avatar Aug 06 '18 22:08 akhti

Thank you for the PR. If I'm not mistaken, the latest release 0.4.1 still has the old way. I would prefer if we could keep compatibility with that - if we have to, we could detect stream function it is in setup.py and pass a custom define. (I'm obviously biased, but on master you should really use PyTorch's own CTC.)

t-vi avatar Aug 08 '18 11:08 t-vi

I see some diclscrepancy in quality between the implementation in the master and warp CTC. But it may be random noise. Still investigating.

Have you tried to use both for some task/benchmark?

On Aug 8, 2018 2:45 PM, "Thomas Viehmann" [email protected] wrote:

Thank you for the PR. If I'm not mistaken, the latest release 0.4.1 still has the old way. I would prefer if we could keep compatibility with that - if we have to, we could detect stream function it is in setup.py and pass a custom define. (I'm obviously biased, but on master you should really use PyTorch's own CTC.)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/t-vi/warp-ctc/pull/9#issuecomment-411377771, or mute the thread https://github.com/notifications/unsubscribe-auth/AHH-m0ZnRa2ZehqqP40ip42ZTTuuC5o2ks5uOs9PgaJpZM4VxM8E .

akhti avatar Aug 09 '18 13:08 akhti

Re PyTorch's own CTC: I've not published anything for that yet. You do have a logsoftmax? The native GPU implementation makes a similar precision / speed tradeoff as cudnn, but it should really be OK. If you want to check on the accuracy for individual examples, you could also use doubles with the native implementation. The CPU version uses log-space-calculations throughout and can be considered to be even more accurate.

For this PR: I'm not going to manage to work on it during the next two weeks, but I would imagine that the setup.py tests whether torch has the new CUDAContext.h header and defines an ifdef OLD_STREAM_API or so if not to keep the old code.

t-vi avatar Aug 10 '18 13:08 t-vi