fastmachinelearning / hls4ml

Machine learning on FPGAs using HLS
https://fastmachinelearning.org/hls4ml
Apache License 2.0
1.17k stars 388 forks source link

No longer make ExponentPrecisionType and XnorPrecisionType inherit from IntegerPrecisionType #845

Closed jmitrevs closed 10 months ago

jmitrevs commented 11 months ago

Description

When trying to propagate precisions, isinstance(var, (IntegerPrecisionType, FixedPrecisionType)) produces incorrect results if the variable is ExponentPrecisionType or XnorPrecisionType. The "is a" requirement for inheritance is violated. Therefore it's better to remove the inheritance, which this PR does. It also cleans up a bit of the remaining string parsing there was and switches from the old % string formatting to f-strings or .format().

Type of change

It is not really a "bug" but the previous inheritance structure caused issues with yet to be added precision propagation.

Tests

This should not break anything. The qkeras tests in particular are the main tests for this.

Checklist

jmitrevs commented 11 months ago

About the pytest failure, Quartus Winograd is broken for Xnor inputs. #804 forces im2col instead of Winograd for this reason. I can hide this failure if desired.

jmitrevs commented 11 months ago

I think after #804 is merged the pytest failure will go away. But I am curious what people think about the "review" comments I made.