Skip to content

Conversation

christopherngutierrez
Copy link
Contributor

Proposed changes

Added rotate kernel, which has been tested and validated to work.

Minor changes (to avoid code replication):

  • Created a helper function for common Polys in Relin/Rotate kernels
  • Added a fix index to the KeyMul, which is different for Relin/Rotate kernels

Types of changes

What types of changes does your code introduce to the HE Toolkit project?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update (if none of the other choices apply)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating
the PR. If you are unsure about any of them, do not hesitate to ask. We are
here to help! This is simply a reminder of what we are going to look for before
merging your code.

  • I have read the CONTRIBUTING agreement
  • Current formatting and unit tests / base functionality passes locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in downstream modules

christopherngutierrez and others added 16 commits July 24, 2024 14:25
…ix enables generate_p_isa_ops_pgm and run_p_isa to scripts in functional modeler to run properly and keeps the naming convention consistant between hec-p-isa-tools and HERACLES-SEAL-isa-mapping.
… problem with relin, which rotate uses partially

Signed-off-by: christopherngutierrez <[email protected]>
kylanerace
kylanerace previously approved these changes Sep 16, 2024
Copy link
Collaborator

@kylanerace kylanerace left a comment

Choose a reason for hiding this comment

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

LGTM, tested and running E2E

@faberga
Copy link
Collaborator

faberga commented Sep 17, 2024

This PR is dependent on PR #43 and should not be merged before it.

add_original.name = self.input0.name

return mixed_to_pisa_ops(
opts = mixed_to_pisa_ops(
Copy link
Collaborator

Choose a reason for hiding this comment

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

This was resolved in the Relin PR but resurface in the Rotate. Sounds as the rotate code has not been rebased correctly.

Add(self.context, self.output, mul_by_rlk_modded_down, add_original),
Comment("End of relin kernel"),
)
return opts
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above.

@faberga faberga marked this pull request as draft September 19, 2024 18:53
@faberga
Copy link
Collaborator

faberga commented Sep 27, 2024

Closed as this PR has been superseded by PR #48

@faberga faberga closed this Sep 27, 2024
@faberga faberga deleted the christopherngutierrez/rotate branch September 27, 2024 20:26
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.

3 participants