Skip to content

Conversation

@tafsiri
Copy link
Contributor

@tafsiri tafsiri commented Sep 9, 2020

8fPPbEdzvwb9jpL

This makes it easier to process the list of kernels for a custom build since ops implemented with customGrad don't have true backend kernels.

To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.


This change is Reviewable

@tafsiri
Copy link
Contributor Author

tafsiri commented Sep 9, 2020

cc @nsthorat in case there is any corner case or other use of the scope names as we had them before.

Copy link
Contributor

@annxingyuan annxingyuan left a comment

Choose a reason for hiding this comment

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

Just to make sure I understand - in the example described in the PR description, the model calls the FusedConv2D kernel and calls fusedConv2D through custom grad?

Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @annxingyuan and @tafsiri)


tfjs/tools/custom_bundle/cli.ts, line 107 at r1 (raw file):

  // kernels so we need to filter them out.
  function isNotCustomOp(kernelName: string) {
    return !kernelName.endsWith('__op');

could we maybe pull out __op into a variable and reference it elsewhere?


tfjs-core/src/ops/operation.ts, line 41 at r1 (raw file):

  }

  // an an __op suffix to distinguish ops from kernels in tf.profile

Add an __op

@tafsiri
Copy link
Contributor Author

tafsiri commented Sep 9, 2020

@annxingyuan fusedConv2d the op is called which starts a scope with the op name (this name won't be replaced by a kernel name when you enter the first engine.runKernelFunc). Later on this op will call another engine.runKernelFunc with FusedConv2d which will also get added to tf.profile (it will start a new scope).

For reference above are the kernels captured for blazeface by tf.profile (sorry I should have put that in the original discussion).

@tafsiri tafsiri merged commit f586a82 into master Sep 10, 2020
@tafsiri tafsiri deleted the op_scope_rename branch September 10, 2020 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants