tflite-support icon indicating copy to clipboard operation
tflite-support copied to clipboard

Migrate away from assertDeepAlmostEqual

Open khanhlvg opened this issue 3 years ago • 6 comments

The reason to use tf.test, is to be able to use https://www.tensorflow.org/api_docs/python/tf/test/TestCase#assertProtoEquals instead of our own implementation.

@kinaryml Could you investigate if you can migrate away from using assertDeepAlmostEqual?

khanhlvg avatar Apr 14 '22 06:04 khanhlvg

@khanhlvg A testing module in TFDS also seems to use a similar variant of assertDeepAlmostEqual here FYI.

I believe it should be fine to use the function since another TF library uses it too.

kinarr avatar Apr 14 '22 14:04 kinarr

On a side note, I've also opened a branch that uses assertProtoEquals to compare results here.

kinarr avatar Apr 14 '22 19:04 kinarr

@kinaryml I've merged #784 with some minor changes. I migrated back to use the mock in the unittest module because the one in TF is an internal API that we aren't supposed to access. Could you merge the latest changes into your branch, resolve conflict and raise a PR? Thanks!

khanhlvg avatar Apr 15 '22 14:04 khanhlvg

@khanhlvg I've created yet another branch here instead and we can review the changes. This should have all the conflicts resolved.

kinarr avatar Apr 15 '22 19:04 kinarr

Thanks Kinar! The changes look good to me. Could you send a PR to the main tflite-support repo so that I can import it and test on internal build system?

khanhlvg avatar Apr 15 '22 23:04 khanhlvg

@khanhlvg I've raised a PR for it in the main tflite-support repo now. PTAL here.

kinarr avatar Apr 16 '22 01:04 kinarr