Skip to content

Remove double transpose for Conv2DBatchnorm#965

Open
jmitrevs wants to merge 2 commits intomainfrom
transpose_to_backend
Open

Remove double transpose for Conv2DBatchnorm#965
jmitrevs wants to merge 2 commits intomainfrom
transpose_to_backend

Conversation

@jmitrevs
Copy link
Copy Markdown
Contributor

@jmitrevs jmitrevs commented Feb 8, 2024

Description

In the layers.py Conv2DBatchnorm makes a transpose if it is the resource strategy, but only for Vivado and VivadoAccelerator backends. Note that Conv2DBatchnorm inherits from Conv2D, and Conv2D makes the same transpose in:

https://github.com/fastmachinelearning/hls4ml/blob/main/hls4ml/backends/vivado/passes/resource_strategy.py#L29-L32

I think therefore the transpose is doubly-applied. I am therefore removing the backend-specific transpose in the layers.py, which regardless does not belong there.

Type of change

  • Bug fix (non-breaking change that fixes an issue)

Tests

I assume there's a Conv2DBatchnorm test that will catch this change, but maybe not.

Test Configuration:

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.

@jmitrevs jmitrevs added the bug label Feb 8, 2024
@jmitrevs
Copy link
Copy Markdown
Contributor Author

jmitrevs commented Feb 9, 2024

This was motivated by issue #957

@jmitrevs
Copy link
Copy Markdown
Contributor Author

Conv2DBatchnorm seems buggy in general. We may want a more serious look at it.

@bo3z bo3z added reference Reference code; unlikely to be merged but can be used as an example/starting point wont-merge PR unlikely to be merged; used mainly as a reference. and removed bug labels Jan 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reference Reference code; unlikely to be merged but can be used as an example/starting point wont-merge PR unlikely to be merged; used mainly as a reference.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants