Add a conversion of individual operations in FQ2I pass.

[Icemist]

This conversion happens after the basic part of FQ2I pass. The main idea of the conversion is to find and transform operations with dequantized inputs one by one individually. Only operations from the allowed list are allowed.

For example, if on the above general pattern op2 is not registered with the FTVMFakeQuantizationToInteger attribute, op1 operation can still be converted. Converted pattern below:

   x    w       x   w
   |    |       |   |
   dq   dq      \   /
    \   /        op1
     op1          |
      |     =>   dq
     op2          |
      |          op2
      |           |
      q           q

Measured accuracy on a BERT model bert_large_v1_1_fake_quant.onnx:

Model type	Accuracy
without old fq2i	{"exact_match": 82.41248817407758, "f1": 90.06966006174216}
with old fq2i	{"exact_match": 82.41248817407758, "f1": 90.06966006174216}
with new fq2i	{"exact_match": 82.73415326395458, "f1": 90.17821060674615}

[masahi]

Can you show some accuracy results on QAT models (BERT etc)?

[masahi]

Please also show the reference result (accuracy without fq2i).

[Icemist]

Please also show the reference result (accuracy without fq2i).

Model type	Accuracy
without old fq2i	{"exact_match": 82.41248817407758, "f1": 90.06966006174216}
with old fq2i	{"exact_match": 82.41248817407758, "f1": 90.06966006174216}
with new fq2i	{"exact_match": 82.73415326395458, "f1": 90.17821060674615}

[mbrookhart]

Overall very nice, only comments are around improved documentation and an optional flag.

@masahi and @elvin-n and I talked a couple of months ago and agreed that the QAT version of the pass (which you're adding here) should probably be a separate pass from the explicit fake quantized model ala tflite and TensorRT. I go back and forth on what the right final form is (I've found edge cases for both versions), but I'd love some improved comments on why this particular design

[mbrookhart]

Could we add some comments and advice about what ops should be included in this list?

[Icemist]

I added a comment of how I selected these operations.

[mbrookhart]

I'm not sure how problematic this will be in non-QAT models, but would it make sense to add another bool to make the "Optional" part of the pass actually optional?

[elvin-n]

let's add parameter enableQAT to the FakeQuantizationToInteger pass with default value not to call QAT transformation

[Icemist]

I added the parameter, but called it use_qat. I'm not sure if CamelCase fits here, and I've seen qat in other libraries shortened to lower case.

[AndrewZhaoLuo]

Need to read pass a little more carefully to understand difference between this and existing fq2i passes

[masahi]

Need to read pass a little more carefully to understand difference between this and existing fq2i passes

The key is that in the models that existing pass expects, there is always matching dq and q. This PR handles cases such assumption doesnt hold.

[AndrewZhaoLuo]

That is understood, my main concern is a lot of code looks similar and I'm wondering if this "optional" pass can supersede the original pass.

[elvin-n]

That is understood, my main concern is a lot of code looks similar

Ideologically transformations look similar, but it is unable to extend current transformation exactly due to its expectation of DQ->subgraph->Q pattern. And it is not a lot of duplication - all op conversion defined through FTVMFakeQuantizationToInteger mechanism is reused, that is great

I'm wondering if this "optional" pass can supersede the original pass.

Interesting question. Probably it can work in that way as well, but should not be so important.

[elvin-n]

@mbrookhart

agreed that the QAT version of the pass (which you're adding here) should probably be a separate pass from the explicit fake quantized model ala tflite and TensorRT

As I see we did exactly what we agreed - the transformation itself is independent from the current one in opposite to the previous my PR (FakeQuantizationRewriter and OptionalFakeQuantizationRewriter). Then we agreed, if I am not mistaken, to have the only pass to simplify user's life. User will be aware about only one function - FakeQuantizeToInteger that he have to call in his python code. On the other hand inside the pass, there will be two transformations - current one and new one. If we want to have QAT transformation optional, ok, let's add parameter to the FakeQuantizeToInteger with default value not to call QAT

[Icemist]

That is understood, my main concern is a lot of code looks similar and I'm wondering if this "optional" pass can supersede the original pass.

I guess this is a slightly different approach. Not all operations are suitable for the new pass.
Some operations like "add" require an explicit out_scale, which in the existing pass is taken from the quantization operation.
This is possible for the existing pass because it goes from the end to the beginning of the selected subgraph.
In the new pass we go from the beginning to the end. Starting from the dequantization operations and have no access to the quantization operation info at the moments of converting. Or maybe even stop halfway before we get the info.

[AndrewZhaoLuo]

LGTM, one more small q