Closed jmitrevs closed 10 months ago
This now matches for io_parallel implementations with the Latency strategy. Resource still fails (potentially for unrelated reasons also on Quartus) and for io_stream. @vloncar , I am curious if you think there's a better way to preserve the XnorPrecision designation when doing MaxPooling than I am doing here. It is a bit related to precision propagation, but I think in this case you really need to preserve the type for the network to make sense.
With the latest push, this works for Vitis and Vivado (but not Quartus). Note that #817 was triggered with the test previously, which has been modified to not trigger the problem. However, a better fix for #817 would be preferred. It is somewhat beyond the scope of this PR, though.
The way Winograd is written it doesn't support the xnor types, so I have forced the Quartus io_parallel tests to use im2col. If we want to update Winograd to do binary, I'll leave that as a separate development.
Don't fully understand why the pytests fail here but succeeded on my laptop. I thought I was using the same random number, but maybe not.
I think the issue was just accumulator precision in the test (big number - almost equal big number) and then quantization based on > or < 0. Let's see if increasing the bitwidth makes the tests succeed.
The way Winograd is written it doesn't support the xnor types, so I have forced the Quartus io_parallel tests to use im2col. If we want to update Winograd to do binary, I'll leave that as a separate development.
Enabling binary support for Winograd needs to change (i) the explicit multiplication for binary weights, to use the product function from nnet_mult
and (ii) for binary inputs, see how the addition of input elements behaves. But Winograd's algorithm is really meant to reduce the number of multiplications/DSP utilisation and with binary CNNs, that's not really a concern, so I don't think it is necessarily important in this case.
I see there are some errors. Will make it draft till I fix them.
The failures seem to be due to precision of the quantization, so I am removing the draft designation
Looks good to me.
The test passed, so I think we can maybe merge this.
Description
The check on the pytest for binary CNNs had to be disabled because it failed. This enables it, and attempts to fix issues that it discovers
Type of change
Tests
The test checking has been enabled. It still fails in most cases.
Checklist
pre-commit
on the files I edited or added.