shiwenloong commented on pull request #6232: URL: https://github.com/apache/incubator-tvm/pull/6232#issuecomment-670922682
> > I think adding `unbiased` argument to variance op will change the consistency. It will also bring more work to isolate the variance op from reduce ops. > > I'm not completely sure what you mean by above, but how about adding `VarianceAttrs` with default `unbiased=false`? I think that should fix the coupling between variance and reduce op you are referring to? > > Anyway, adding two new ops just for the sake of adding `unbiased` support sounds a bit overkill to me. If we follow this modification method, we need to add more than just `VarianceAttrs`. Many codes and tests related to the variance operator need to be changed. For example, we should also add `variance_shape_func` in `python/tvm/relay/op/_reduce.py`. `std` and `variance` should be isolated from `python/tvm/relay/op/reduce.py` because their parameters are different from other reduce ops. In my opinion, although adding two unbiased operators seems a bit redundant, it has little impact on the code structure and is of a lower probability of causing bugs. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
