Remove extra generics from op generation#193
Conversation
Signed-off-by: Ryan Nett <rnett@calpoly.edu>
Signed-off-by: Ryan Nett <rnett@calpoly.edu>
Signed-off-by: Ryan Nett <rnett@calpoly.edu>
Signed-off-by: Ryan Nett <rnett@calpoly.edu>
karllessard
left a comment
There was a problem hiding this comment.
Great job @rnett , I like this improvement. Just a few nits to look and we are good to merge.
tensorflow-core/tensorflow-core-api/src/bazel/op_generator/op_specs.cc
Outdated
Show resolved
Hide resolved
tensorflow-core/tensorflow-core-api/src/bazel/op_generator/op_specs.cc
Outdated
Show resolved
Hide resolved
tensorflow-core/tensorflow-core-api/src/bazel/op_generator/op_specs.cc
Outdated
Show resolved
Hide resolved
tensorflow-core/tensorflow-core-api/src/bazel/op_generator/op_specs.cc
Outdated
Show resolved
Hide resolved
tensorflow-core/tensorflow-core-api/src/bazel/op_generator/op_specs.h
Outdated
Show resolved
Hide resolved
|
Following your comments about missing files, it is not a concern. When we upgrade the TF runtime version, we need to delete the For example, |
|
@karllessard it seems like it is, but to be sure: will the CI do op generation, or should I re-run it locally to check the fixes? |
|
The CI will regenerate the op and I've triggered a full native build on that PR to make sure we don't break anything, I'll have to wait until it is finished before merging |
* Successfully remove extra type params, but it broke javadoc generation Signed-off-by: Ryan Nett <rnett@calpoly.edu> * Generate covariant types Signed-off-by: Ryan Nett <rnett@calpoly.edu> * Do generation Signed-off-by: Ryan Nett <rnett@calpoly.edu> * Update help text. Signed-off-by: Ryan Nett <rnett@calpoly.edu> * Fixes Signed-off-by: Ryan Nett <rnett@calpoly.edu>
* Initial checkin * Initial checkin and sync with master * Initial checkin and sync with master * JavaDoc cleanup * Javadoc fixes * Change LossInterface to LossMetric. Fix JavaDoc, modify one line code block to include braces. * Removed hashmap for variables, they are not needed as the variables only live within a single instance of a Metric. * reformat code * Add tests for assertBroadcastable * Change type to resultType * Added V data type for sampleWeights so that it is not forced to be the same type as the return or internal variables, * change 'type' to 'resultType' * clean up mean and fix assert assertBroadcastable * fix error message * Change sampleWeights to have its own generic type <S extends TNumber> * Add commment about invalid tests expecting IllegalArgumentExceptions * Add this exception instead of the more generic IllegalArgumentException when static shapes cannot boradcast. * change IllegalArgumentException to NotBroadcastableException. change hasValidNonscalarShape to canBroadcastNonscalarShapes change hasValidNonscalarShape to canBroadcastNonscalarShapes * reformat code * Fis=x Javadoc move the dynamic shapes and rank down to the dynamic section so they are created needlessly when static Fix if statement to check for unknown size and unknown dimensions * Fix Reduce to use boradcastWeights, renamed WeightBroadcastTest to AssertBroadcastableTest and added BroadcastWeightsTest * Added comment to count to indicate that it may be weighted. * Added SetsOps and fixed AssertBroadcastable to use SetsOps methods, * Fixed based on various PR comments. * Deleted, no longer needed after change to Variable handling in Metrics. * Nicer error messages for mode-forbidden ops (#169) * start fobbiden ops checks Signed-off-by: Ryan Nett <rnett@calpoly.edu> * fix style Signed-off-by: Ryan Nett <rnett@calpoly.edu> * move checks to builder method Signed-off-by: Ryan Nett <rnett@calpoly.edu> * Initialization imprvements (#178) * No-op on initAdd in eager mode Signed-off-by: Ryan Nett <rnett@calpoly.edu> * runInit() method in session Signed-off-by: Ryan Nett <rnett@calpoly.edu> * add doInitialization() to Runner Signed-off-by: Ryan Nett <rnett@calpoly.edu> * fix javadoc Signed-off-by: Ryan Nett <rnett@calpoly.edu> * assume only graph or eager environments Signed-off-by: Ryan Nett <rnett@calpoly.edu> * Remove doInit(), update javadocs Signed-off-by: Ryan Nett <rnett@calpoly.edu> * small fixes Signed-off-by: Ryan Nett <rnett@calpoly.edu> * Clairify tensorOf lifetime requirements (#190) * Clairify tensorOf lifetime requirements Signed-off-by: Ryan Nett <rnett@calpoly.edu> * Do codegen Signed-off-by: Ryan Nett <rnett@calpoly.edu> * Remove extra generics from op generation (#193) * Successfully remove extra type params, but it broke javadoc generation Signed-off-by: Ryan Nett <rnett@calpoly.edu> * Generate covariant types Signed-off-by: Ryan Nett <rnett@calpoly.edu> * Do generation Signed-off-by: Ryan Nett <rnett@calpoly.edu> * Update help text. Signed-off-by: Ryan Nett <rnett@calpoly.edu> * Fixes Signed-off-by: Ryan Nett <rnett@calpoly.edu> * Add Java 11 support - Initial Phase (#185) * Add profile for JDK11 and Automatic-Module-Name to jars * add maven.compiler.release=11 * Update manual ops for new codegen (#196) Signed-off-by: Ryan Nett <rnett@calpoly.edu> * Fix Losses to use CHANNELS_FIRST/LAST for CategoricalCrossentropy * Fix SetOps to properly convert sparse tensor to dense tensor using tf.sparse.sparseToDense with the output of tf.sparse.denseToDenseSetOperation * Initial checkin * Initial checkin and sync with master * Initial checkin and sync with master * JavaDoc cleanup * Javadoc fixes * Change LossInterface to LossMetric. Fix JavaDoc, modify one line code block to include braces. * Removed hashmap for variables, they are not needed as the variables only live within a single instance of a Metric. * reformat code * Add tests for assertBroadcastable * Change type to resultType * Added V data type for sampleWeights so that it is not forced to be the same type as the return or internal variables, * change 'type' to 'resultType' * clean up mean and fix assert assertBroadcastable * fix error message * Change sampleWeights to have its own generic type <S extends TNumber> * Add commment about invalid tests expecting IllegalArgumentExceptions * Add this exception instead of the more generic IllegalArgumentException when static shapes cannot boradcast. * change IllegalArgumentException to NotBroadcastableException. change hasValidNonscalarShape to canBroadcastNonscalarShapes change hasValidNonscalarShape to canBroadcastNonscalarShapes * reformat code * Fis=x Javadoc move the dynamic shapes and rank down to the dynamic section so they are created needlessly when static Fix if statement to check for unknown size and unknown dimensions * Fix Reduce to use boradcastWeights, renamed WeightBroadcastTest to AssertBroadcastableTest and added BroadcastWeightsTest * Added comment to count to indicate that it may be weighted. * Added SetsOps and fixed AssertBroadcastable to use SetsOps methods, * Fixed based on various PR comments. * Deleted, no longer needed after change to Variable handling in Metrics. * Fix Losses to use CHANNELS_FIRST/LAST for CategoricalCrossentropy * Fix SetOps to properly convert sparse tensor to dense tensor using tf.sparse.sparseToDense with the output of tf.sparse.denseToDenseSetOperation Co-authored-by: Ryan Nett <rnett@calpoly.edu>
* Successfully remove extra type params, but it broke javadoc generation Signed-off-by: Ryan Nett <rnett@calpoly.edu> * Generate covariant types Signed-off-by: Ryan Nett <rnett@calpoly.edu> * Do generation Signed-off-by: Ryan Nett <rnett@calpoly.edu> * Update help text. Signed-off-by: Ryan Nett <rnett@calpoly.edu> * Fixes Signed-off-by: Ryan Nett <rnett@calpoly.edu>
Fixes #176. I leave a type parameter in if it is used by two or more inputs or outputs.