[uniqueqamar]

Description

Relates to #1592.

This PR proposes a workaround for a Vitis HLS 2025.1 issue where the line buffer used by streaming 2D pooling may be synthesized with depth 1, causing incorrect RTL behavior for 2×2 MaxPooling2D.

In hls4ml/templates/vitis/nnet_utils/nnet_pooling_stream.h, both line-buffer declarations are changed from:

MAX(CONFIG_T::pool_height - 1, 1)

to:

MAX(CONFIG_T::pool_height - 1, 2)

This ensures the declared buffer has at least two rows while preserving the existing indexing logic. The workaround has not yet been validated through Vitis RTL co-simulation.

A regression test was also added in test/pytest/test_pooling.py to inspect the generated header and verify that both declarations use the corrected bound.

Type of change

• Bug fix (non-breaking)
• New feature
• Breaking change
• Documentation
• Refactor

Affected areas

Backend: Vitis

Frontend: Not frontend-specific; the regression test uses Keras.

Components:

• C++ templates / HLS sources under hls4ml/templates/
• Core IR / optimizer
• Packaging / CI

Configurations affected and exercised

Backend io_type Strategy Verified
Vitis io_stream Not strategy-specific Static source inspection only; no full project tests or synthesis

No new configuration options are introduced.

Impact on generated HLS

• This PR changes generated HLS.

The line-buffer declaration now reserves at least two rows. The intended numerical behavior is unchanged, but bit-exactness has not been established.

Latency, initiation interval, LUT, FF, DSP and BRAM utilization have not been measured.

C simulation, synthesis, RTL co-simulation and hardware validation have not been performed.

Tests

Added test_vitis_pooling2d_stream_line_buffer_depth in test/pytest/test_pooling.py.

The test generates a Vitis io_stream 2×2 MaxPooling2D project and checks the resulting nnet_pooling_stream.h for the two updated buffer declarations.

Validation performed:

• Inspected the GitHub diff and both buffer declarations.
• Previously performed isolated C++ checks using a simplified buffer implementation.

Not performed:

• Execution of the new pytest test
• Full hls4ml test suite
• Pre-commit checks
• Vitis HLS C simulation, synthesis or RTL co-simulation

Suggested regression-test command (not yet executed):

pytest test/pytest/test_pooling.py -k vitis_pooling2d_stream_line_buffer_depth -v

Breaking changes and migration

No intended public API changes or migration requirements.

AI assistance disclosure

• None
• Assisted
• Substantial
• Agentic — produced largely end-to-end by an AI agent from my prompts.

Tool: OpenAI ChatGPT.

Usage: C++ implementation, regression-test preparation, isolated checks and PR documentation.

Verification and responsibility:

• I have personally reviewed and understood the entire contribution and take responsibility for it.
• I have verified the generated code against actual hls4ml and HLS semantics.
• The reported test evidence is limited to checks actually performed; no synthesis results are claimed.
• I confirm the right to submit the contribution under the project’s licence.
• I have verified that no AI tool is credited as a commit author.

Unchecked items require personal confirmation or additional validation.

Checklist

• Read the contributing guidelines.
• Run pre-commit on edited files.
• Add a regression test under test/pytest.
• Execute the regression test and verify failure on main and success on this branch.
• Complete full-diff self-review.
• Include AI assistance disclosure.

Release note

Adjust Vitis streaming 2D pooling line-buffer declarations to avoid a reported depth-1 synthesis issue for 2×2 pooling.

Follow-up: Conv2D and DepthwiseConv2D

Following @vloncar’s review, this PR now extends the minimum-two-row line-buffer workaround to Conv2D and DepthwiseConv2D. Two declarations in nnet_conv2d_stream.h and one in nnet_sepconv2d_stream.h now use MAX(CONFIG_T::filt_height - 1, 2).

Source-level regression checks have been added in test/pytest/test_vitis_stream_line_buffer.py, but have not yet been executed.

@vloncar reports successful validation with Vitis HLS 2025.1, 2024.1 and 2023.2, including unchanged latency, initiation interval and timing. These are reviewer-reported results, not independently reproduced by me.

[JanFSchulte]

Please fill in the PR description following the provided template. Otherwise this is very cryptic.

[uniqueqamar]

Please fill in the PR description following the provided template. Otherwise this is very cryptic.

Thank you for pointing this out, I’ve updated the PR description following the template, including the motivation, implementation details, affected configurations, testing status, and AI assistance disclosure.

The Vitis RTL co-simulation and synthesis validation are still pending, and I’ve made that explicit in the description. I appreciate the feedback!

[DaiYukichi]

Thanks for picking this up, @uniqueqamar. I ran C/RTL co-simulation with this PR's header and it fixes the issue.

How I tested it. I took hls4ml/templates/vitis/nnet_utils/nnet_pooling_stream.h from the PR head (9177e6d). Compared with hls4ml 1.3.0, the only differences are the two line_buffer declarations (plus the comment). I dropped that header into projects generated with hls4ml 1.3.0 and ran csim + synth + cosim with Vitis HLS 2025.1 (Windows 11, vitis-run --mode hls --tcl build_prj.tcl).

Results

Design	csim vs RTL, unpatched	csim vs RTL, this PR
Reproducer from #1592 (single 2×2 MaxPooling2D, 16×16×1)	40/64 values differ	0/64
Full U-Net (2 MaxPooling2D layers, 64×64 output)	MAE ≈ 0.12, max 0.686	0/4096, bit-exact

• csynth (estimate, xczu7ev): reproducer 313 FF / 813 LUT (unpatched: 265 / 781); full U-Net +2,496 FF / +1,536 LUT vs unpatched. Same latency in both cases (515 and 348,759–348,814 cycles).
• The RTL output, resources and latency are identical to my local workaround from pooling2d_cl (io_stream) line buffer synthesized with depth 1 in Vitis HLS 2025.1 → C/RTL co-simulation mismatch for MaxPooling2D 2×2 #1592.

Notes

• Patching templates/vitis/ is correct: that is the header the Vitis backend uses. (My diff in pooling2d_cl (io_stream) line buffer synthesized with depth 1 in Vitis HLS 2025.1 → C/RTL co-simulation mismatch for MaxPooling2D 2×2 #1592 pointed to templates/vivado/ by mistake. That file has the same MAX(CONFIG_T::pool_height - 1, 1) pattern, but I have only tested Vitis HLS 2025.1, so I can't say whether Vivado HLS is affected.)
• Changing the compute_pool_buffer_2d parameter as well keeps the declarations consistent. My local patch only changed the static declaration, and both give the same RTL.
• The new test only checks the string, so it guards against a revert but not against the HLS behaviour itself. Since cosim isn't part of the regular CI, that seems like a reasonable compromise to me, but I'll defer to the maintainers.

Happy to rerun anything if the PR changes.

[uniqueqamar]

Thanks for picking this up, @uniqueqamar. I ran C/RTL co-simulation with this PR's header and it fixes the issue.

How I tested it. I took hls4ml/templates/vitis/nnet_utils/nnet_pooling_stream.h from the PR head (9177e6d). Compared with hls4ml 1.3.0, the only differences are the two line_buffer declarations (plus the comment). I dropped that header into projects generated with hls4ml 1.3.0 and ran csim + synth + cosim with Vitis HLS 2025.1 (Windows 11, vitis-run --mode hls --tcl build_prj.tcl).

Results

Design csim vs RTL, unpatched csim vs RTL, this PR
Reproducer from #1592 (single 2×2 MaxPooling2D, 16×16×1) 40/64 values differ 0/64
Full U-Net (2 MaxPooling2D layers, 64×64 output) MAE ≈ 0.12, max 0.686 0/4096, bit-exact

• csynth (estimate, xczu7ev): reproducer 313 FF / 813 LUT (unpatched: 265 / 781); full U-Net +2,496 FF / +1,536 LUT vs unpatched. Same latency in both cases (515 and 348,759–348,814 cycles).
• The RTL output, resources and latency are identical to my local workaround from pooling2d_cl (io_stream) line buffer synthesized with depth 1 in Vitis HLS 2025.1 → C/RTL co-simulation mismatch for MaxPooling2D 2×2 #1592.

Notes

• Patching templates/vitis/ is correct: that is the header the Vitis backend uses. (My diff in pooling2d_cl (io_stream) line buffer synthesized with depth 1 in Vitis HLS 2025.1 → C/RTL co-simulation mismatch for MaxPooling2D 2×2 #1592 pointed to templates/vivado/ by mistake. That file has the same MAX(CONFIG_T::pool_height - 1, 1) pattern, but I have only tested Vitis HLS 2025.1, so I can't say whether Vivado HLS is affected.)
• Changing the compute_pool_buffer_2d parameter as well keeps the declarations consistent. My local patch only changed the static declaration, and both give the same RTL.
• The new test only checks the string, so it guards against a revert but not against the HLS behaviour itself. Since cosim isn't part of the regular CI, that seems like a reasonable compromise to me, but I'll defer to the maintainers.

Happy to rerun anything if the PR changes.

Thank you so much for running the C simulation, synthesis, and RTL co-simulation tests and sharing such detailed results!

It's great to see that the fix produces bit-exact RTL output for both the minimal reproducer and the full U-Net. I also appreciate your clarification about the Vitis versus Vivado templates and the regression test's limitations.

I'll look into the failing GitLab CI checks and update the PR description to reference your validation results. Thanks again for taking the time to verify this!

[uniqueqamar]

I investigated the two failing GitLab CI jobs (pytest.qkeras and pytest.qkerasV3).

Both report an MD5 checksum mismatch while downloading/loading the OpenML dataset hls4ml_lhc_jets_hlf.arff in test_qkeras.py::test_accuracy[resource].

The remaining tests largely passed (333 passed in pytest.qkeras and 286 passed in pytest.qkerasV3).

These failures appear to be related to the external dataset or CI cache rather than the Vitis pooling changes in this PR.

Could a maintainer advise whether these jobs should be retried with a clean dataset cache, or whether this is a known upstream CI issue?

Thank you!

[JanFSchulte]

You can ignore these failures. We are investigating them in the background. Is there a reason this PR is still marked as a draft?

[uniqueqamar]

Thank you for the clarification! I kept the PR in draft because I wanted to address the CI failures before requesting review.
Since you've confirmed that the CI failures are being investigated separately..I'll mark the PR as ready for review.
Thankyou for your guidance!!

[vloncar]

Thanks for the fix. I checked it with Vitis HLS 2025.1, 2024.1 and 2023.2, the bug only shows up on 2025.1, and the PR fixes it there. Latency, interval and timing are unchanged. The extra FF and LUT are just the line buffer that 2025.1 was dropping, and the results end up where 2024.1 already is. On 2024.1 and 2023.2 the change makes no difference at all.

Conv2D and DepthwiseConv2D with a 2x2 kernel have the same problem on 2025.1, with almost every output wrong. The same change fixes them: the two line_buffer declarations in nnet_conv2d_stream.h and the one in nnet_sepconv2d_stream.h, using MAX(CONFIG_T::filt_height - 1, 2). I tested those too, and they also make no difference on the older versions. Could you add them to this PR?

[uniqueqamar]

Thanks for the fix. I checked it with Vitis HLS 2025.1, 2024.1 and 2023.2, the bug only shows up on 2025.1, and the PR fixes it there. Latency, interval and timing are unchanged. The extra FF and LUT are just the line buffer that 2025.1 was dropping, and the results end up where 2024.1 already is. On 2024.1 and 2023.2 the change makes no difference at all.

Conv2D and DepthwiseConv2D with a 2x2 kernel have the same problem on 2025.1, with almost every output wrong. The same change fixes them: the two line_buffer declarations in nnet_conv2d_stream.h and the one in nnet_sepconv2d_stream.h, using MAX(CONFIG_T::filt_height - 1, 2). I tested those too, and they also make no difference on the older versions. Could you add them to this PR?

Thank you for testing this across multiple Vitis HLS versions and confirming that the issue also affects both Conv2D and DepthwiseConv2D.

I’ve updated both headers with the requested fixes, added regression tests covering the affected cases, and revised the PR description to document the additional changes and validation results.

I really appreciate your feedback and help!

[JanFSchulte]

pre-commit.ci autofix