Add support for unipolar bconv2d inputs.

[AdamHillier]

This is very much a draft PR.

What do these changes do?

This is a converter-only change that adds support for binary convolutions with unipolar inputs -- all credit is given to @jneeven who had the original idea for the trick that this PR takes advantage of and that allows this to be a converter-only change, i.e. that avoids the need to modify the kernels. I've tried to explain the method in the code comment above the pattern I've added in prepare_patterns.td.

Specifically, it adds support for models with the following input quantiser:

class UnipolarSteSign(lq.quantizers.SteSign):
    def __init__(self, clip_value: float = 0.5, **kwargs):
        super().__init__(clip_value=clip_value, **kwargs)

    def call(self, inputs):
        return 0.5 * super().call(inputs - 0.5) + 0.5

The end2end test results seem to indicate that the method works.

I'm publishing this as an early draft because I am keen to solicit feedback on whether this is a sensible approach; if we want to progress further we will need to add the quantiser to Larq.

Still to do:

• [ ] Add file-check tests.
• [ ] Add a UnipolarSteSign quantiser to Larq.
• [ ] Do a validation run with a unipolar model and check that we get the expected accuracy.

How Has This Been Tested?

The end2end tests have been modified to test unipolar inputs. These pass locally, and hopefully on CI too, with the much tighter ouput bounds introduced by #524.

Benchmark Results

N/A.

Related issue number

N/A.

[lgeiger]

That's really cool, I completely forgot about this 🎉

In larq we already have the unipolar SteHeaviside quantizer (although I think the backward pass should actually be a [0, 1] clip instead of the [-1, 1] clipping which is currently used). The forward pass is implemented in lq.math.heaviside as tf.sign(tf.nn.relu(x)) so we could reuse this implementation without needing to modify larq itself.

[Tombana]

I'm confused as to why the end2end test works with ReLu. Basically we have the following:

UnipolarDotProd = -0.5 * BinaryDotProd + constant

(per output-channel, and the constant depends on the output channel). We can compute BinaryDotProd and then fuse the -0.5 and constant into the post_activation_{multiplier_bias}. However if there's a ReLu then for the unipolar conv, Keras expects ReLu(UnipolarDotProd). But

ReLu(UnipolarDotProd) != -0.5 * ReLu( BinaryDotProd ) + constant

where the right-hand-side is what the bconv op would compute.

If we don't write it as ReLu but as a clamp with arbitrary clamp values, it might work (I'm not 100% sure, it might become channel-dependent). So maybe a solution would be to let the converter compute clamp-threshold values instead of an enum with 'None, ReLu, ...'. Previously we argued against this because it'd expose implementation details to the API. However if it would solve this issue we might want to rethink that. We could also add a flag unipolar = {False/True} to the API and then the op itself could recompute those clamp threshold values.

[AdamHillier]

I'm confused as to why the end2end test works with ReLu.

Ah yes, I should have mentioned this in the PR. Doing this transformation prevents ReLU fusing, because it adds a non-trivial bias, and we only fuse ReLUs when the bconv2d has a zero bias. So the reason the test passes is that the ReLU isn't fused.

[jneeven]

That's really cool, I completely forgot about this tada

In larq we already have the unipolar SteHeaviside quantizer (although I think the backward pass should actually be a [0, 1] clip instead of the [-1, 1] clipping which is currently used). The forward pass is implemented in lq.math.heaviside as tf.sign(tf.nn.relu(x)) so we could reuse this implementation without needing to modify larq itself.

The DoReFaQuantizer can be used for unipolar activations, and I think we did so without any modifications at all (simply set the precision to 1). However, we empirically found LSQ works better, but haven't added it to Larq yet. Since LSQ only has a learnable scale parameter and no offset, there should be no difference between the two in terms of inference (this is obviously only true for binary LSQ, for multi-bit it starts getting funkier), and it probably make more sense to use either of these quantizers than to add a new UnipolarSteSign.

Very nice that this is finally happening though!

[AdamHillier]

That's really cool, I completely forgot about this 🎉

In larq we already have the unipolar SteHeaviside quantizer (although I think the backward pass should actually be a [0, 1] clip instead of the [-1, 1] clipping which is currently used). The forward pass is implemented in lq.math.heaviside as tf.sign(tf.nn.relu(x)) so we could reuse this implementation without needing to modify larq itself.

Ah cool, I hadn't realised that, so that relies on the fact that tf.sign(0) = 0? Doesn't that make it a bit weird in the sense that without lq.math.sign it doesn't generate a quant/dequant pair. I'm definitely happy to investigate using a definition like that though.

[CNugteren]

Closing this one due to inactivity. We can re-open later if someone wants to work on this.