Skip to content

Add Cropping layers support #1309

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

HamzaEzzRa
Copy link

Description

This PR adds support for Keras' Cropping1D and Cropping2D layers with Vivado and Vitis backend in both io_parallel and io_stream modes.

The implementation is straightforward and follows a similar style to zero-padding implementations. I added tests for Vivado and Vitis backends.

I have yet to run this on a board, but implementation was successful targeting a ZCU102, using Vivado 2019.1. Although this type of layer has a fairly niche usage, I still found a use case for it, and it could benefit the community.

Linked Issue: #1308

Type of change

  • New feature (non-breaking change which adds functionality)

Tests

I added unit tests in test/pytest/test_cropping.py

Test Configuration:

  • io_type: ["io_stream", "io_parallel"]
  • backend: ["Vivado", "Vitis"]
  • model_type: ["1d", "2d"]

Checklist

  • I have read the guidelines for contributing.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have installed and run pre-commit on the files I edited or added.
  • I have added tests that prove my fix is effective or that my feature works.

@HamzaEzzRa
Copy link
Author

pre-commit.ci autofix

@jmitrevs jmitrevs added the please test Trigger testing by creating local PR branch label Jun 12, 2025
Copy link
Contributor

@JanFSchulte JanFSchulte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test error are unrelated to this PR. Looks good to me.

@JanFSchulte
Copy link
Contributor

@calad0i Do you think this needs to be included explicitly also in the keras v3 parser, or should we let the fallback take care of it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
please test Trigger testing by creating local PR branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants