One: [Compiler/luci] 'quantized_dimension' info not passed

Created on 16 Jul 2020  路  8Comments  路  Source: Samsung/ONE

For CWQ*, quantized_dimension in tflite/circle schema is essential.

(*Channle-wise quantization, or per-axis quantization. See tensorflow/quantization_spec.md)

https://github.com/Samsung/ONE/blob/74682f8bd5f74dddf46254ec81f7762e276e4d0f/nnpackage/schema/circle_schema.fbs#L83-L91

Issue

But as I understand, quantized_dimension is not dealt with in luci or any compiler projects yet.

$ git grep quantized_dimension -- compiler

compiler/luci-interpreter/include/luci_interpreter/core/Tensor.h:// TODO Add 'quantized_dimension' field for per-channel case when IR provides it.
compiler/mir/src/mir_tflite_importer/schema/schema.fbs:  quantized_dimension:int;

https://github.com/Samsung/ONE/blob/74682f8bd5f74dddf46254ec81f7762e276e4d0f/compiler/luci/import/src/CircleReader.cpp#L152-L173
https://github.com/Samsung/ONE/blob/74682f8bd5f74dddf46254ec81f7762e276e4d0f/compiler/luci/export/src/CircleTensorExporter.cpp#L305

As I understand, we can imply this value from operator type as a work-around(e.g. 3 for DEPTHWISE_CONV_2D, 0 for CONV_2D), but this approach might be dangerous.

All 8 comments

As I understand, we can imply this value from operator type as a work-around(e.g. 3 for DEPTHWISE_CONV_2D, 0 for CONV_2D), but this approach might be dangerous.

FYI, circle-quantizer is taking the above approach. https://github.com/Samsung/ONE/blob/master/compiler/luci/pass/src/QuantizationUtils.cpp#L108

I've listed modules affected by the addition of quantized_dimension.

luci/lang
luci/import
luci/export
luci/pass (QuantizeWithMinMax)
luci-interpreter (AffineQuantization)
tflite2circle
circledump
tfldump
tflchef
circlechef

There can be more..

To support channel-wise quantization (including quantized_dimension) in luci-interpreter, we need to change class Tensor.

Currently, Tensor has following functions.

  float scale() const
  {
    assert(_quantization.scale.size() == 1);
    return _quantization.scale[0];
  }

  float zero_point() const
  {
    assert(_quantization.zero_point.size() == 1);
    return _quantization.zero_point[0];
  }

The above functions should return a vector, not the single element. Or, I think it is ok to return just AffineQuantization*.
@s-barannikov what do you think of this?

I think it is ok to return just AffineQuantization*

OK to me, but the existing functions should remain. They are useful for U8 quantization, which is always per-layer.

OK to me, but the existing functions should remain. They are useful for U8 quantization, which is always per-layer.

In fact, circle-quantizer supports U8 CWQ (channel-wise quantization) for some Ops. (https://github.com/Samsung/ONE/blob/master/compiler/luci/pass/src/QuantizeWithMinMaxPass.cpp#L478)

But, luci-interpreter does not have a kernel for U8 CWQ, so I'd like to leave luci-interpreter as-is, until the interpreter supports U8 CWQ kernels.

In fact, circle-quantizer supports U8 CWQ (channel-wise quantization) for some Ops.

Let's use the new interface for those ops.

The above functions should return a vector, not the single element.

I suggest to add two new functions zero_points() and scales() (note the s suffix) which would return vectors. I think it is even better than just return AffineQuantization.

Can we close this? or still active?

Can we close this? or still active?

I think no more issues remain. Let's close this issue.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

mhs4670go picture mhs4670go  路  3Comments

wateret picture wateret  路  4Comments

mhs4670go picture mhs4670go  路  3Comments

KimDongEon picture KimDongEon  路  4Comments

hasw7569 picture hasw7569  路  4Comments