Skip to content
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

[ntuple] some fixes to Real32Quant #17252

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

silverweed
Copy link
Contributor

@silverweed silverweed commented Dec 11, 2024

  • fix wrong calculation for maxErr in ntuple_packing
  • fix wrong emin/emax adjustment in UnquantizeReals
  • ensure min and max values of ValueRange are normal

Notes

This should fix an assertion failure observed on AArch64 by @hahnjo.
These bugs do not have the potential of generating corrupted data (barring users that pass exotic floats as min/max values), but they can trigger false positives of the "out of range" assertion failures.

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

- fix wrong calculation for maxErr in ntuple_packing
- fix wrong emin/emax adjustment in UnquantizeReals
- ensure min and max values of ValueRange are normal
@hahnjo
Copy link
Member

hahnjo commented Dec 11, 2024

This should fix an assertion failure observed on AArch64 by @hahnjo.

Thanks, confirmed on macphsft28 (mac13arm). Afterwards, we probably want to turn on asserts for PR builds...

Copy link

Test Results

    17 files      17 suites   3d 22h 50m 42s ⏱️
 2 662 tests  2 661 ✅ 0 💤 1 ❌
43 594 runs  43 592 ✅ 0 💤 2 ❌

For more details on these failures, see this check.

Results for commit 71b5758.

@silverweed silverweed merged commit c0ecad4 into root-project:master Dec 12, 2024
17 of 21 checks passed
@silverweed silverweed deleted the ntuple_quant_fixes branch December 12, 2024 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants