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

Fix EltwiseReduceMod #90

Merged
merged 9 commits into from Nov 1, 2021
Merged

Fix EltwiseReduceMod #90

merged 9 commits into from Nov 1, 2021

Conversation

fboemer
Copy link
Contributor

@fboemer fboemer commented Oct 29, 2021

Fixes #86

@fboemer fboemer closed this Oct 29, 2021
@fboemer fboemer reopened this Oct 29, 2021
@fboemer fboemer marked this pull request as ready for review October 29, 2021 23:24
@fboemer fboemer requested a review from a team as a code owner October 29, 2021 23:24
@fboemer
Copy link
Contributor Author

fboemer commented Oct 29, 2021

@bryan113, thanks for reporting this. Are you able to check if this PR fixes the SEAL issue microsoft/SEAL#411?

Copy link
Contributor

@joserochh joserochh left a comment

Choose a reason for hiding this comment

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

LG

Copy link
Contributor

@jlhcrawford jlhcrawford left a comment

Choose a reason for hiding this comment

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

Was there a particular reason you didn't choose to parameterise the existing tests?

Also I don't think you needed to change the test fixture name to have the "Test" suffix if you wanted to keep it consistent with the non-parameterised tests.

Otherwise, this looks good to me.

@bryan113
Copy link

bryan113 commented Nov 1, 2021

I tested SEAL with HEXL commit: 42f057d and it appears to be working now. Thank you!

@WeiDaiWD
Copy link

WeiDaiWD commented Nov 2, 2021

@fboemer Any plan to release another version of HEXL?

@fboemer
Copy link
Contributor Author

fboemer commented Nov 2, 2021

@fboemer Any plan to release another version of HEXL?

@WeiDaiWD, yes we'll plan to release v1.2.3 with this fix in a day or two.

@lidh15 lidh15 mentioned this pull request Nov 3, 2021
fboemer added a commit that referenced this pull request Nov 8, 2021
* Fix AVVX512DQ EltwiseReduceMod
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.

EltwiseReduceMod Fails with Larger Moduli on 1.2.2 With DQ Processor
5 participants