-
Notifications
You must be signed in to change notification settings - Fork 202
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
automatically generate op convenience overloads? #246
Comments
Here's another example a little further down in the same code. Python: math_ops.maximum(dp, 0) Java: tf.math.maximum(dP, tf.dtypes.cast(tf.constant(0), dP.type())) |
The slicing can be cleaned up a bit with the new One potential generation strategy for ops like maximum is to look at the supported types for the arguments, and if it supports all of More generally though, this is something I'd really like to do, especially for However, generating these isn't really possible, since they aren't specified anywhere in the op defs. All we know for say If we want to do something like this, it would have to be done manually. I still think it's worth doing at some point, and we can clean up some of the other api idiosyncrasies at the same time (like |
One simple way to manually curate an API would be to start now, choosing the location and pattern, and fill it out as we code. We could choose a pattern that would be relatively amenable to being completed at a later time, such as by organizing the methods in such a way that one of us could eventually sweep through the whole list of ops and fill in the blanks. |
Alternatively, we could advocate into the main TensorFlow project that we add enough information to the protobuf files to generate high-usability statically typed APIs? I wouldn't expect this challenge to be unique to Java. |
Or, another alternative, we could create our own set of protobuf files that add this information, and then gradually populate those as we code. Over time, we could advocate pushing that information upstream into the main protobuf files. |
I like the protobuf options more, since it's a lot easier to generate Kotlin wrappers off of it. Even just optional If we do end up doing our own version, I'd want to heavily use annotations to mark things like parameters that should be receivers so that I can generate matching idiomatic Kotlin APIs without having to maintain 2 curated APIs. |
My leaning is that the sooner we start this, the less painful it will be, because we have an opportunity to do it -- and tweak it -- as we write our own code. Combining that with @rnett's point about preferring protobuf to hand-code, perhaps the best starting point would be adding our own protobuf files that augment (1 for 1) main tensorflow's files, and that allow us to better tune our codegen to Java and Kotlin? I expect we'd eventually want to advocate for pushing (some or all of) our new metadata upstream, but that would be a slower process and might make more sense once we had goodness to demonstrate? |
Agreed about starting soon. I'm reconsidering the "protobuf is better" a bit though. There's non-straightforward things like |
We may want to consider combining this with #255 in some way, as they are a bit similar. |
Wow, I'm increasingly appreciating that this is a large design space with lots of conflicting goals and constraints. Would it make the most sense for someone to draft a design document? One approach could be doing this as a |
#264 is an excellent motivating example. See transpose being under linalg (but not in python?), no I think a design doc would make sense. Could do it as an issue like I did for functions if we don't want to start adding docs, but I do like the idea of being able to PR it. I don't have the time to work on that at the moment, but I can look it over and make sure it would work with the Kotlin generation. |
Should we extend the code generation for ops to automatically generate convenience overloads? This would help reduce the gap between Python notation and Java notation.
As an example of where we stand now, here's some Python code (keras/metrics.py, around line 2203):
And here's the corresponding Java code (AUC.java, around line 809):
The text was updated successfully, but these errors were encountered: