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

Support cuDNN CTC functions #1769

Merged
merged 14 commits into from Apr 8, 2019
Merged

Support cuDNN CTC functions #1769

merged 14 commits into from Apr 8, 2019

Conversation

aonotas
Copy link
Contributor

@aonotas aonotas commented Nov 1, 2018

Add cuDNN CTC functions.
This cuDNN CTC is supported from v7.

  • cudnnCreateCTCLossDescriptor
  • cudnnDestroyCTCLossDescriptor
  • cudnnSetCTCLossDescriptor
  • cudnnGetCTCLossDescriptor
  • cudnnGetCTCLossWorkspaceSize
  • cudnnCTCLoss

@aonotas aonotas changed the title upport ctc functions Support ctc functions Nov 1, 2018
@aonotas aonotas changed the title Support ctc functions Support cuDNN CTC functions Nov 1, 2018
@okuta okuta self-assigned this Nov 2, 2018
cupy/cudnn.pyx Outdated
@@ -382,6 +382,13 @@ def set_dropout_descriptor(desc, handle, dropout):
cudnn.setDropoutDescriptor(desc.value, handle, dropout, 0, 0, 0)


def create_ctc_loss_descriptor(data_type):
Copy link
Member

Choose a reason for hiding this comment

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

I want to hide descriptor and wrap cuDNN interface because raw cuDNN API is too complex.

This is high level API example.

def activation_forward(core.ndarray x, int mode, double coef=0.0):

Cloud you remove this function from this PR?

Copy link
Contributor Author

@aonotas aonotas Nov 2, 2018

Choose a reason for hiding this comment

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

OK. Thank you for your comment.
Do you mean should I define the other high level API function like following?

# in cupy/cudnn.pyx
def ctc_loss(data_type, other_args_for_cudnnCTCLoss):
    # create descriptor
    desc = Descriptor(cudnn.createCTCLossDescriptor(),
                    py_cudnn.destroyCTCLossDescriptor)
    cudnn.setCTCLossDescriptor(desc.value, data_type)

     # compute workspace
     getCTCLossWorkspaceSize(hoge, worksize)
     # allocate worksize
     workspace = ...
     # compute CTC loss
      CTCLoss(hoge, workspace, other_args_for_cudnnCTCLoss)
      return loss, gradients

Copy link
Member

Choose a reason for hiding this comment

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

Yes, something like that.

Copy link
Member

@okuta okuta Dec 24, 2018

Choose a reason for hiding this comment

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

Cloud you rename this function or delete this?

Suggested change
def create_ctc_loss_descriptor(data_type):
def _create_ctc_loss_descriptor(data_type):

cupy/cuda/cudnn.pyx Outdated Show resolved Hide resolved
cupy/cuda/cudnn.pyx Outdated Show resolved Hide resolved
@okuta
Copy link
Member

okuta commented Nov 2, 2018

jenkins, test this please.

@chainer-ci
Copy link
Member

Jenkins CI test (for commit 2309250, target branch master) failed with status FAILURE.
(For contributors, please wait until the reviewer confirms the details of the error.)

okuta and others added 2 commits November 2, 2018 12:06
Co-Authored-By: aonotas <nanigashi03@gmail.com>
Co-Authored-By: aonotas <nanigashi03@gmail.com>
@aonotas aonotas force-pushed the cudnn-ctc branch 2 times, most recently from 7b26f2d to c206bc6 Compare November 2, 2018 04:30
@aonotas
Copy link
Contributor Author

aonotas commented Nov 6, 2018

Can you check this comments when you can time?
#1769 (comment)

@aonotas
Copy link
Contributor Author

aonotas commented Nov 12, 2018

In ctc function, I found these things:
In these two functions, we should set CPU pointer (labelLengths, inputLengths, labels).

  • cudnnGetCTCLossWorkspaceSize
  • cudnnCTCLoss
    when we set GPU pointer, it occurs 'segment fault'.

so I define arguments wiht CPU pointer in high-level API.
https://github.com/cupy/cupy/pull/1769/files#diff-da3b471a8f2bebf46bb07243ffb2caedR392

Please give me some advice.

and Do you know why this error occurs?
https://travis-ci.org/cupy/cupy/jobs/453816083#L846

@unnonouno
Copy link
Contributor

In such case, using numpy.ndarray as an interface is enough in my opinion. > CPU pointer

@unnonouno
Copy link
Contributor

You need to fix the dummy header file?

@okuta okuta added cat:feature New features/APIs to-be-backported Pull-requests to be backported to stable branch labels Dec 3, 2018
@okuta okuta added this to the v6.0.0b2 milestone Dec 3, 2018
cupy/cudnn.pyx Outdated
@@ -382,6 +382,13 @@ def set_dropout_descriptor(desc, handle, dropout):
cudnn.setDropoutDescriptor(desc.value, handle, dropout, 0, 0, 0)


def create_ctc_loss_descriptor(data_type):
Copy link
Member

@okuta okuta Dec 24, 2018

Choose a reason for hiding this comment

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

Cloud you rename this function or delete this?

Suggested change
def create_ctc_loss_descriptor(data_type):
def _create_ctc_loss_descriptor(data_type):

@okuta
Copy link
Member

okuta commented Dec 24, 2018

jenkins, test this please.

@chainer-ci
Copy link
Member

Jenkins CI test (for commit 02df19a, target branch master) failed with status FAILURE.
(For contributors, please wait until the reviewer confirms the details of the error.)

@hvy hvy modified the milestones: v6.0.0b2, v6.0.0b3 Jan 24, 2019
@beam2d beam2d removed this from the v6.0.0b3 milestone Feb 28, 2019
@kmaehashi
Copy link
Member

@aonotas Do you have some time to update this PR?

@chainer-ci
Copy link
Member

Jenkins CI test (for commit 02df19a, target branch master) failed with status FAILURE.
(For contributors, please wait until the reviewer confirms the details of the error.)

@aonotas
Copy link
Contributor Author

aonotas commented Apr 5, 2019

I fix the code (just rename the function).
Please review it! I'm sorry for my late PR.

@okuta
Copy link
Member

okuta commented Apr 5, 2019

chainerci please

@okuta
Copy link
Member

okuta commented Apr 5, 2019

jenkis, test this please.

@chainer-ci
Copy link
Member

Jenkins CI test (for commit cb658d8, target branch master) failed with status FAILURE.

@okuta okuta removed the to-be-backported Pull-requests to be backported to stable branch label Apr 8, 2019
@okuta okuta added this to the v7.0.0a1 milestone Apr 8, 2019
@okuta
Copy link
Member

okuta commented Apr 8, 2019

LGTM!

@okuta okuta merged commit b40438e into cupy:master Apr 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat:feature New features/APIs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants