Skip to content

Conversation

@jzhoulon
Copy link
Contributor

Adding more attribution C API for pluggable device kernels implementation.

@google-ml-butler google-ml-butler bot added the size:L CL Change Size: Large label Oct 14, 2020
@google-cla google-cla bot added the cla: yes label Oct 14, 2020
@jzhoulon
Copy link
Contributor Author

@annarev @penpornk @yisitu We have added some kernel C API for retrieving more op attributions. Can you help to review it? thanks.

@gbaned gbaned self-assigned this Oct 14, 2020
@gbaned gbaned requested a review from annarev October 14, 2020 16:12
@gbaned gbaned added the awaiting review Pull request awaiting review label Oct 16, 2020
@tensorflowbutler tensorflowbutler removed the awaiting review Pull request awaiting review label Oct 23, 2020
@gbaned gbaned requested a review from annarev October 27, 2020 16:55
@gbaned gbaned added the awaiting review Pull request awaiting review label Oct 27, 2020
@tensorflowbutler tensorflowbutler removed the awaiting review Pull request awaiting review label Oct 31, 2020
@gbaned gbaned requested review from annarev and removed request for annarev November 2, 2020 11:05
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Nov 5, 2020
@annarev annarev added the API review API Review label Nov 5, 2020
@annarev
Copy link
Contributor

annarev commented Nov 5, 2020

Looks good to me. I added API review label so that API owners can take a look as well

@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Nov 5, 2020
@google-ml-butler google-ml-butler bot removed the ready to pull PR ready for merge process label Nov 5, 2020
@gbaned gbaned requested a review from annarev November 5, 2020 09:54
@gbaned gbaned added the awaiting review Pull request awaiting review label Nov 9, 2020
@gbaned
Copy link
Contributor

gbaned commented Nov 25, 2020

@jzhoulon Can you please resolve conflicts? Thanks!

@jzhoulon
Copy link
Contributor Author

@jzhoulon Can you please resolve conflicts? Thanks!

@gbaned I have resolved conflicts.

@gbaned gbaned requested a review from annarev November 26, 2020 03:21
Copy link
Contributor

@guptapriya guptapriya left a comment

Choose a reason for hiding this comment

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

(for tf-api-owners)

//
// If the attribute could not be found or could not be interpreted as
// bool, *status is populated with an error.
TF_CAPI_EXPORT extern void TF_OpKernelConstruction_GetAttrString(
Copy link
Contributor

Choose a reason for hiding this comment

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

So should the type of val be char* val instead?

@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Dec 5, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Dec 5, 2020
Copy link
Contributor

@guptapriya guptapriya left a comment

Choose a reason for hiding this comment

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

for @tensorflow/api-owners

@google-ml-butler google-ml-butler bot added the kokoro:force-run Tests on submitted change label Dec 7, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Dec 7, 2020
@google-ml-butler google-ml-butler bot removed the ready to pull PR ready for merge process label Dec 8, 2020

// Helper macros for the TF_OpKernelConstruction_GetAttr* tests.
#define EXPECT_TF_SIZE(attr_name, expected_list_size, expected_total_size) \
\
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this line. It just contains \

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Dec 8, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Dec 8, 2020
@gbaned gbaned removed the ready to pull PR ready for merge process label Dec 8, 2020
@gbaned gbaned requested a review from annarev December 8, 2020 13:46
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Dec 8, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Dec 8, 2020
@copybara-service copybara-service bot merged commit 798304c into tensorflow:master Dec 9, 2020
copybara-service bot pushed a commit that referenced this pull request Dec 10, 2020
…ieving attributes

PiperOrigin-RevId: 346844467
Change-Id: I9d2121c3ea2402df51879852d0bade21577b3d9b
copybara-service bot pushed a commit that referenced this pull request Dec 11, 2020
…el_c_api

PiperOrigin-RevId: 347042144
Change-Id: I05c7587c7a491199f8ba24109eaf4f6d1bc83c72
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API review API Review cla: yes ready to pull PR ready for merge process size:L CL Change Size: Large

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants