Generate op input and attribute accessors by rnett · Pull Request #302 · tensorflow/java

rnett · 2021-04-27T00:48:51Z

This adds a nested class to the generated op classes that contains typesafe (and not quite type safe) accessors for the op inputs and attributes. It's only supported for graph ops at the moment, but it will eventually support ForwardOperations as well (from the eager gradient support). The main use for this is defining custom gradients, whether it's the legacy graph gradients or the new gradient API.

I haven't committed the generated outputs to make it easier to review, I'll do so after approval or in another PR.

rnett · 2021-04-27T20:17:17Z

The formatting issues were since my IDE somehow got off google-java-format. I have the plugin now, which added some things that weren't in the XML.

karllessard · 2021-04-28T02:36:20Z

Looks like the javadoc reformatting is still present

rnett · 2021-04-28T03:55:22Z

Yeah, that happened when I applied the google-java-format plugin, and it matches the spec. Idk why the XML format didn't do it, I can revert to that.

We really should set up some kind of CI formatter, I keep getting burnt by this. I haven't had time to look at it much but one of the plugins you linked in the lint PR looked like it did ktlint too (although it had a bug), that would be nice since we could to the Kotlin API in the same plugin.

Signed-off-by: Ryan Nett <rnett@calpoly.edu>

Signed-off-by: Ryan Nett <JNett96@gmail.com>

Signed-off-by: Ryan Nett <rnett@calpoly.edu>

Signed-off-by: Ryan Nett <JNett96@gmail.com>

rnett · 2021-09-24T16:37:32Z

Reminder for myself: run generation before merging! I'm going to work on a CI test for that.

rnett · 2021-09-28T17:12:14Z

FYI @karllessard I'm going to be out of town most of the next 2 weeks or so. I might have time to follow up on these PRs, but I'm not sure. Feel free to take things over. I should at least be able to reply to comments.

Signed-off-by: Ryan Nett <JNett96@gmail.com>

karllessard

Thanks @rnett !

karllessard · 2021-10-15T01:08:08Z

@rnett , can you please fix the conflict with Names so I can merge?

rnett · 2021-10-15T01:09:46Z

Let me generate the op files first, too.

rnett · 2021-10-15T01:21:15Z

I've been toying with the idea of a CI check that checks git status after running build to ensure that there's no un-commited codegen, thoughts?

karllessard · 2021-10-15T01:27:47Z

I've been toying with the idea of a CI check that checks git status after running build to ensure that there's no un-commited codegen, thoughts?

That's interesting, indeed! Shouldn't be hard to do. Though it looks like we still have this non-deterministic order of the *Ops fields in the main Ops class, or at least I do. We'll need to fix that first.

Signed-off-by: Ryan Nett <JNett96@gmail.com>

rnett · 2021-10-15T01:30:06Z

Yeah, good point, I'll look at that first.

karllessard · 2021-10-15T01:36:41Z

Looks good, I've just merged another PR (#388) though so I'll wait that one to complete building before merging this one.

karllessard · 2021-10-15T20:41:12Z

@rnett , I've just realized that we forgot to a CI build round on this PR before merging it... :(

Failing on Windows: https://github.com/tensorflow/java/runs/3908491275?check_suite_focus=true

It fails on Linux too but it looks more like a bug we are having with the Bazel cache and that was happening before.

CC\ @saudet

rnett · 2021-10-15T20:44:21Z

I think I know what that is, I just need to skip a bunch of things that aren't exported on windows.

rnett changed the title ~~Generatr op input and attribute accessors~~ Generate op input and attribute accessors Apr 27, 2021

Craigacp reviewed Apr 27, 2021

View reviewed changes

rnett force-pushed the rn_generatr_op_inputs branch from 002fa85 to faca5f4 Compare April 27, 2021 20:17

rnett mentioned this pull request Jun 17, 2021

Init Scope #338

Merged

rnett force-pushed the rn_generatr_op_inputs branch from 4436f16 to e3f4561 Compare July 16, 2021 02:28

rnett mentioned this pull request Jul 17, 2021

Add method to copy graph operation to new graph or eager operation builder #355

Open

rnett requested a review from Craigacp July 19, 2021 20:48

rnett mentioned this pull request Aug 27, 2021

Graph custom gradient support #292

Merged

rnett and others added 19 commits September 18, 2021 16:09

Attribute accessors

a9c8171

Signed-off-by: Ryan Nett <rnett@calpoly.edu>

Base class for accessors

8b78370

Signed-off-by: Ryan Nett <rnett@calpoly.edu>

Add attr value getter

2fc0e32

Signed-off-by: Ryan Nett <rnett@calpoly.edu>

Java side metadata

4b4421d

Signed-off-by: Ryan Nett <rnett@calpoly.edu>

More accessors

71e0748

Signed-off-by: Ryan Nett <rnett@calpoly.edu>

Input accessors

43401cd

Signed-off-by: Ryan Nett <rnett@calpoly.edu>

Generator for input class

cd8bc9a

Signed-off-by: Ryan Nett <rnett@calpoly.edu>

Fix some native issues

0504feb

Signed-off-by: Ryan Nett <rnett@calpoly.edu>

CommentFixes

681be63

Signed-off-by: Ryan Nett <JNett96@gmail.com>

Fix formatting

8f6af9f

Signed-off-by: Ryan Nett <JNett96@gmail.com>

Fix formatting again

0c237f3

Signed-off-by: Ryan Nett <JNett96@gmail.com>

More format fixes

ac58d31

Signed-off-by: Ryan Nett <JNett96@gmail.com>

Rebase updates

63af4a6

Signed-off-by: Ryan Nett <JNett96@gmail.com>

Add AttrValue setter

dcce378

Signed-off-by: Ryan Nett <rnett@calpoly.edu>

Map tensorflow/c/eager/c_api_experimental.h

8bde900

Signed-off-by: Ryan Nett <rnett@calpoly.edu>

Eager proto attribute setter

4ed6429

Signed-off-by: Ryan Nett <rnett@calpoly.edu>

Make opHandle final

7f9092b

Signed-off-by: Ryan Nett <rnett@calpoly.edu>

Fix format

b8da4a2

Signed-off-by: Ryan Nett <rnett@calpoly.edu>

Rebase fixes

0895e48

Signed-off-by: Ryan Nett <JNett96@gmail.com>

rnett force-pushed the rn_generatr_op_inputs branch from a32b212 to 0895e48 Compare September 18, 2021 23:15

karllessard reviewed Sep 28, 2021

View reviewed changes

Comment thread tensorflow-core/tensorflow-core-api/src/main/java/org/tensorflow/GraphOperation.java

rnett added 4 commits October 12, 2021 19:31

Extract attribute getters to a helper class

16c6167

Signed-off-by: Ryan Nett <JNett96@gmail.com>

Use new attribute getters in codegen

874ebcc

Signed-off-by: Ryan Nett <JNett96@gmail.com>

Comment explaining helper class

51a8df8

Signed-off-by: Ryan Nett <JNett96@gmail.com>

Add device getter

e00ce2b

Signed-off-by: Ryan Nett <JNett96@gmail.com>

rnett requested a review from karllessard October 14, 2021 16:12

karllessard previously approved these changes Oct 15, 2021

View reviewed changes

Merge branch 'master' into rn_generatr_op_inputs

21ac57c

rnett dismissed karllessard’s stale review via 21ac57c October 15, 2021 01:09

Fix formatting and generate ops

bbd3a1f

Signed-off-by: Ryan Nett <JNett96@gmail.com>

karllessard approved these changes Oct 15, 2021

View reviewed changes

karllessard merged commit 05d9de3 into tensorflow:master Oct 15, 2021

rnett deleted the rn_generatr_op_inputs branch October 17, 2021 00:42

karllessard mentioned this pull request Oct 20, 2021

Metrics init scope #382

Merged

Conversation

rnett commented Apr 27, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rnett commented Apr 27, 2021

Uh oh!

karllessard commented Apr 28, 2021

Uh oh!

rnett commented Apr 28, 2021

Uh oh!

rnett commented Sep 24, 2021

Uh oh!

Uh oh!

rnett commented Sep 28, 2021

Uh oh!

karllessard left a comment

Choose a reason for hiding this comment

Uh oh!

karllessard commented Oct 15, 2021

Uh oh!

rnett commented Oct 15, 2021

Uh oh!

rnett commented Oct 15, 2021

Uh oh!

karllessard commented Oct 15, 2021

Uh oh!

rnett commented Oct 15, 2021

Uh oh!

karllessard commented Oct 15, 2021

Uh oh!

karllessard commented Oct 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rnett commented Oct 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

karllessard commented Oct 15, 2021 •

edited

Loading