Skip to content

fix: QuantizationInfo equality operator overload by treating implicit offset as zero#1293

Open
morgolock wants to merge 1 commit into
mainfrom
pr/fix_pool_layer_perf
Open

fix: QuantizationInfo equality operator overload by treating implicit offset as zero#1293
morgolock wants to merge 1 commit into
mainfrom
pr/fix_pool_layer_perf

Conversation

@morgolock

@morgolock morgolock commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Update QuantizationInfo::operator== to compare normalized uniform() values when both operands are effectively per-layer, while preserving strict vector equality for per-channel quantization info.

This makes equivalent qinfo such as QuantizationInfo(1.0f) and QuantizationInfo(1.0f, 0) compare equal, avoiding incorrect dispatch differences for paths that rely on QuantizationInfo equality.

Add UNIT coverage for equivalent uniform qinfo, differing uniform qinfo, and per-channel equality behavior, and remove the pooling-specific regression coverage.

Change-Id: I54ab2cc9b2fae810b9d2767676097858eb8621c8

@morgolock morgolock requested a review from gunes-arm June 16, 2026 13:19

@gunes-arm gunes-arm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please link tickets if any

Comment thread src/cpu/kernels/internal/CpuPool2dAssemblyWrapperKernel.cpp Outdated
@morgolock morgolock force-pushed the pr/fix_pool_layer_perf branch from 6c1271a to 338544f Compare June 18, 2026 13:52
@morgolock morgolock requested a review from gunes-arm June 18, 2026 13:53

@gunes-arm gunes-arm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

fix: treat equivalent uniform QuantizationInfo as equal

Please start with capital letter, i.e. "fix: Treat ..."

Also, please change the PR title and description accordingly.

Comment thread tests/validation/UNIT/QuantizationInfo.cpp Outdated
Comment thread tests/validation/UNIT/QuantizationInfo.cpp
Comment thread src/cpu/kernels/internal/CpuPool2dAssemblyWrapperKernel.cpp
@gunes-arm

gunes-arm commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

fix: treat equivalent uniform QuantizationInfo as equal

Please start with capital letter, i.e. "fix: Treat ..."

Also, please change the PR title and description accordingly.

Also, what do you think about the following commit message. It rings more bells when read in release notes:?

fix: QuantizationInfo equality operator overload by treating implicit offset as zero

@morgolock

Copy link
Copy Markdown
Contributor Author

fix: treat equivalent uniform QuantizationInfo as equal
Please start with capital letter, i.e. "fix: Treat ..."
Also, please change the PR title and description accordingly.

Also, what do you think about? It rings more bells when read in release notes:?

fix: QuantizationInfo equality operator overload by treating implicit offset as zero

Yes, sorry, I had made the mental link between the request and the release notes but somehow I forgot it again :)

@morgolock morgolock force-pushed the pr/fix_pool_layer_perf branch from 338544f to e13b7e1 Compare June 19, 2026 10:15
@morgolock morgolock changed the title fix: Treat equivalent pooling qinfo as non-requantized fix: Treat equivalent uniform QuantizationInfo as equal Jun 19, 2026
@morgolock morgolock requested a review from gunes-arm June 19, 2026 10:16
@gunes-arm

gunes-arm commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

fix: treat equivalent uniform QuantizationInfo as equal
Please start with capital letter, i.e. "fix: Treat ..."
Also, please change the PR title and description accordingly.

Also, what do you think about the following commit message. It rings more bells when read in release notes:?

fix: QuantizationInfo equality operator overload by treating implicit offset as zero

What's your opinion on the commit message title? (+ PR description ought to change).
Do you have tickets that you can refer to in the description?

@morgolock

morgolock commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

fix: treat equivalent uniform QuantizationInfo as equal
Please start with capital letter, i.e. "fix: Treat ..."
Also, please change the PR title and description accordingly.

Also, what do you think about the following commit message. It rings more bells when read in release notes:?
fix: QuantizationInfo equality operator overload by treating implicit offset as zero

What's your opinion on the commit message title? (+ PR description ought to change). Do you have tickets that you can refer to in the description?

Apologies, sorted now. I don't have a ticket for this.

@gunes-arm

Copy link
Copy Markdown
Contributor

fix: treat equivalent uniform QuantizationInfo as equal
Please start with capital letter, i.e. "fix: Treat ..."
Also, please change the PR title and description accordingly.

Also, what do you think about the following commit message. It rings more bells when read in release notes:?
fix: QuantizationInfo equality operator overload by treating implicit offset as zero

What's your opinion on the commit message title? (+ PR description ought to change). Do you have tickets that you can refer to in the description?

Apologies, sorted now. I don't have a ticket for this.

Thanks a lot Pablo. You didn't say what you think about the proposed commit message title.

… offset as zero

Update QuantizationInfo::operator== to compare normalized uniform() values when both operands are effectively per-layer, while preserving strict vector equality for per-channel quantization info.

This makes equivalent qinfo such as QuantizationInfo(1.0f) and QuantizationInfo(1.0f, 0) compare equal, avoiding incorrect dispatch differences for paths that rely on QuantizationInfo equality.

Add UNIT coverage for equivalent uniform qinfo, differing uniform qinfo, and per-channel equality behavior, and remove the pooling-specific regression coverage.

Signed-off-by: Pablo Marquez Tello <pablo.tello@arm.com>
Change-Id: I54ab2cc9b2fae810b9d2767676097858eb8621c8
@morgolock morgolock force-pushed the pr/fix_pool_layer_perf branch from e13b7e1 to 27eb4b8 Compare June 19, 2026 11:02
@morgolock morgolock changed the title fix: Treat equivalent uniform QuantizationInfo as equal fix: QuantizationInfo equality operator overload by treating implicit offset as zero Jun 19, 2026
@morgolock

Copy link
Copy Markdown
Contributor Author

fix: treat equivalent uniform QuantizationInfo as equal
Please start with capital letter, i.e. "fix: Treat ..."
Also, please change the PR title and description accordingly.

Also, what do you think about the following commit message. It rings more bells when read in release notes:?
fix: QuantizationInfo equality operator overload by treating implicit offset as zero

What's your opinion on the commit message title? (+ PR description ought to change). Do you have tickets that you can refer to in the description?

Apologies, sorted now. I don't have a ticket for this.

Thanks a lot Pablo. You didn't say what you think about the proposed commit message title.

Changed the title now to fix: QuantizationInfo equality operator overload by treating implicit offset as zero for both: commit and PR

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants