thesps / conifer

Fast inference of Boosted Decision Trees in FPGAs
Apache License 2.0
48 stars 27 forks source link

Add expression balancing for xilinxhls backend #68

Closed francescobrivio closed 6 months ago

francescobrivio commented 6 months ago

PR Description

This PR is addressing issue https://github.com/thesps/conifer/issues/66. Main changes:

These changes bring a notable improvements in latency (and partially in resource consumption) when using the AP_SAT flag in the BDT ScorePrecision, while maintaining performances identical to the current version of conifer in terms of BDT score.

Additionally, in commit bffd3a6125e31f68cce29ffefeb1a00d32343b8f I fixed a typo in the "Development instructions" and added pandas to the dev_requirements.txt file.

PR Validation

The following tests are done with:

Latency/Resources table:

Conifer version ScorePrecision vsynth LUT vsynth FF Latency
master ap_fixed<11,4,AP_RND_CONV,AP_SAT> 51936 5032 133
This PR ap_fixed<11,4,AP_RND_CONV,AP_SAT> 51163 4489 9
-------- -------- -------- -------- --------
master ap_fixed<11,4,AP_RND_CONV> 43329 3542 4
This PR ap_fixed<11,4,AP_RND_CONV> 42988 3800 4

Score plots:

thesps commented 6 months ago

Thanks for this nice contribution! I've checked on some other models and see consistent improvements as you have shown. The tests are passing too (well, as much as they are passing on master branch...).

I will therefore merge this!