tensorflow / tensorflow

An Open Source Machine Learning Framework for Everyone
https://tensorflow.org
Apache License 2.0
185.67k stars 74.19k forks source link

Convert FloorModOp to tosa #57520

Open YellowHCH opened 2 years ago

YellowHCH commented 2 years ago
Click to expand! ### Issue Type Bug ### Source source ### Tensorflow Version 2.9.1 ### Custom Code No ### OS Platform and Distribution Linux Ubuntu 20.04 ### Mobile device _No response_ ### Python version 3.9 ### Bazel version 5.1 ### GCC/Compiler version 9 ### CUDA/cuDNN version _No response_ ### GPU model and memory _No response_ ### Current Behaviour? ```shell from source code: https://github.com/tensorflow/tensorflow/blob/master/tensorflow/compiler/mlir/tosa/transforms/legalize_common.cc#L2457 floormod(x, y) = x / y - floor(x / y) from doc:https://tensorflow.google.cn/api_docs/python/tf/raw_ops/FloorMod?version=nightly floormod(x, y) = x - floor(x / y) * y I wonder if I misunderstood ``` ### Standalone code to reproduce the issue ```shell tosa ir I get: ' module attributes {tf.versions = {bad_consumers = [], min_consumer = 0 : i32, producer = 561 : i32}, tf_saved_model.semantics} { func.func @serving_default(%arg0: tensor<11x10xf32> {tf_saved_model.index_path = ["model_input1"]}, %arg1: tensor {tf_saved_model.index_path = ["model_input2"]}) -> (tensor<11x10xf32> {tf_saved_model.index_path = ["model_output"]}) attributes {tf.entry_function = {control_outputs = "", inputs = "model_input1:0,model_input2:0", outputs = "FloorMod:0"}, tf_saved_model.exported_names = ["serving_default"]} { %0 = "tosa.reciprocal"(%arg1) : (tensor) -> tensor %1 = "tosa.reshape"(%0) {new_shape = [1, 1]} : (tensor) -> tensor<1x1xf32> %2 = "tosa.mul"(%arg0, %1) {shift = 0 : i32} : (tensor<11x10xf32>, tensor<1x1xf32>) -> tensor<11x10xf32> %3 = "tosa.floor"(%2) : (tensor<11x10xf32>) -> tensor<11x10xf32> %4 = "tosa.sub"(%2, %3) : (tensor<11x10xf32>, tensor<11x10xf32>) -> tensor<11x10xf32> return %4 : tensor<11x10xf32> } } ' ``` ### Relevant log output _No response_
mohantym commented 2 years ago

Hi @gadagashwini ! Could you look at this issue. Thank you!

gadagashwini commented 2 years ago

HI @YellowHCH, As per the https://github.com/tensorflow/tensorflow/blob/master/tensorflow/compiler/mlir/tosa/transforms/legalize_common.cc#L2461-L2467

// FloorMod lowering:
  // (1/rhs * lhs) - floor(1/rhs * lhs)

As per the doc https://tensorflow.google.cn/api_docs/python/tf/raw_ops/FloorMod?version=nightly floormod(x, y) = x - floor(x / y) * y

Both are same. Thank you!

youbenye commented 2 years ago

Hi @gadagashwini I noticed that there is a TF FloorMod reference implement here: https://github.com/tensorflow/tensorflow/blob/master/tensorflow/lite/kernels/internal/reference/floor_mod.h#L29

In my point of view,floormod(x, y) = x - floor(x / y) * y is equivalent to the implementation of above, but not equivalent to (1/rhs * lhs) - floor(1/rhs * lhs), seems it missing a multiplier rhs i means rhs * ( (1/rhs * lhs) - floor(1/rhs * lhs) )

Please correct me if I'm wrong. thanks!

google-ml-butler[bot] commented 2 years ago

This issue has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Thank you.

google-ml-butler[bot] commented 2 years ago

Closing as stale. Please reopen if you'd like to work on this further.

google-ml-butler[bot] commented 2 years ago

Are you satisfied with the resolution of your issue? Yes No

gadagashwini commented 2 years ago

Hi @YellowHCH, rhs is missing.Thank you

sachinprasadhs commented 1 year ago

Thanks for opening the issue. The implemenatation of raw_ops.floor_div as mentioned in the document is right way and it internally callsgen_math_opscode file, which does not use the logic of https://github.com/tensorflow/tensorflow/blob/master/tensorflow/compiler/mlir/tosa/transforms/legalize_common.cc#L2461-L2467, However, I have created a PR to changed the implementation of mlir/tosa floor_div.

jpienaar commented 1 year ago

Sounds like this issue was resolved, please reopen if not.

google-ml-butler[bot] commented 1 year ago

Are you satisfied with the resolution of your issue? Yes No

Arunarun001 commented 10 months ago

Hello