CKKS square and power functions #100

philomath213 · 2020-07-13T16:19:40Z

Description

Implement square and power functions for CKKS vector with minimal multiplication depth.
Now it's possible to compute the power of a CKKS vector using .power method or ** operator.
The power function compute recursively the result by splitting the computation into two balanced tree, as a result we will have a result ciphertext with minimal multiplication depth.
There is a special case when the power is zero, in this case a new CKKS vector of ones is returned, while keeping the same size.

Checklist

I have followed the Contribution Guidelines and Code of Conduct
I have commented my code following the OpenMined Styleguide
I have labeled this PR with the relevant Type labels
My changes are covered by tests

bcebere · 2020-07-13T16:55:06Z

tenseal/tensors/ckksvector.cpp

+    return *this;
+}
+
+CKKSVector CKKSVector::power(int power) {


why not unsigned int power?

yeah, you are right, unsigned int will remove the overhead of checking the negative values of power.

bcebere · 2020-07-13T16:58:20Z

tenseal/tensors/ckksvector.cpp

+    int closest_power_of_2 = 1 << static_cast<int>(floor(log2(power)));
+    power -= closest_power_of_2;
+    if (power == 0) {
+        this->power_inplace(closest_power_of_2 / 2)


Wouldn't be better to cache this->power_inplace(closest_power_of_2 / 2) result? It seems that is called twice for the same vector.

Or this->power_inplace(closest_power_of_2 / 2).square() ?

I thought it will require one more multiplication depth, but it won't.
in this case this->power_inplace(closest_power_of_2 / 2).square_inplace() would be better.

youben11 · 2020-07-13T17:19:14Z

tenseal/tensors/ckksvector.cpp

+CKKSVector& CKKSVector::square_inplace() {
+    this->context->evaluator->square_inplace(this->ciphertext);
+
+    if (this->context->auto_relin()) {
+        this->context->evaluator->relinearize_inplace(
+            this->ciphertext, *this->context->relin_keys());
+    }
+
+    if (this->context->auto_rescale()) {
+        this->context->evaluator->rescale_to_next_inplace(this->ciphertext);
+        this->ciphertext.scale() = this->init_scale;
+    }
+
+    return *this;
+}


why not calling mul_inplace which should take care of relin/rescale?

calling mul_inplace requires coping the ciphertext (i.e. this->mul_inplace(*this)).

youben11 · 2020-07-13T17:25:33Z

tenseal/tensors/ckksvector.cpp

+    int closest_power_of_2 = 1 << static_cast<int>(floor(log2(power)));
+    power -= closest_power_of_2;
+    if (power == 0) {
+        this->power_inplace(closest_power_of_2 / 2)
+            .mul_inplace(this->power(closest_power_of_2 / 2));
+    } else {
+        this->power_inplace(power).mul_inplace(this->power(closest_power_of_2));
+    }
+    return *this;


We can use the compute_polynomial_term from utils, we can refactor the name, but calling it with the power, coeffcient as 1, and the precomputed squares of x should compute the power with the most efficient circuit possible.

compute_polynomial_term is particular case of power which have extra overhead.
I think keeping power function as it is would be better for both efficiency and readability.

youben11 · 2020-07-13T17:31:43Z

tests/tensors/test_ckks_vector.py

+    "plain_vec, power",
+    [
+        ([], 2),
+        ([0], 3),
+        ([0, 1, -1, 2, -2], 0),
+        ([1, -1, 2, -2], 1),
+        ([1, -1, 2, -2], 2),
+        ([1, -1, 2, -2], 3),
+        ([1, -2, 3, -4], 1),
+        ([1, -2, 3, -4], 2),
+        ([1, -2, 3, -4], 3),


raising to the power of 4 should be also possible with the default parameters, we need a test-case for this to make sure the circuit is optimum. Same for the other tests

Yeah, this should work with power of 4 as well.

still missing a testcase of 4 here

bcebere

LGTM!

youben11 · 2020-07-13T18:58:30Z

tenseal/binding.cpp

+        .def("square", &CKKSVector::square)
+        .def("square_", &CKKSVector::square_inplace)
+        .def("square_inplace", &CKKSVector::square_inplace)
+        .def("pow", &CKKSVector::power)
+        .def("pow_", &CKKSVector::power_inplace)
+        .def("pow_inplace", &CKKSVector::power_inplace)


The python API have have this inplace operations terminated by a _ (e.g. add_) only and we aren't providing function_name_inplace kind of methods.

youben11 · 2020-07-14T05:47:56Z

tenseal/tensors/ckksvector.cpp

+    // if the power is zero, return a new encrypted vector of ones
+    if (power == 0) {
+        vector<double> ones(this->size(), 1);
+        *this = CKKSVector(this->context, ones, this->init_scale);


I think you can here just replace the ciphertext (ciphertext = this->encrypt(...)) instead of creating a whole new CKKSVector.

tenseal/tensors/ckksvector.cpp

youben11 · 2020-07-14T05:51:24Z

tests/tensors/test_ckks_vector.py

+    "plain_vec, power",
+    [
+        ([], 2),
+        ([0], 3),
+        ([0, 1, -1, 2, -2], 0),
+        ([1, -1, 2, -2], 1),
+        ([1, -1, 2, -2], 2),
+        ([1, -1, 2, -2], 3),
+        ([1, -2, 3, -4], 1),
+        ([1, -2, 3, -4], 2),
+        ([1, -2, 3, -4], 3),


still missing a testcase of 4 here

Co-authored-by: Ayoub Benaissa <ayouben9@gmail.com>

* implement square and power functions for CKKS vector * python binding for CKKS vector square and power functions * tests for CKKS vector square and power functions * replace x.mul_inplace(x) with x.square() * zero power * requested changes * add more test-case with power of 4 * remove _inplace function from python api * add more test-case with power of 4 * fix power_inplace Co-authored-by: Ayoub Benaissa <ayouben9@gmail.com> * more tests to cover all function branches * more tests * python linting Co-authored-by: Ayoub Benaissa <ayouben9@gmail.com>

philomath213 added 5 commits July 13, 2020 16:48

implement square and power functions for CKKS vector

419f7eb

python binding for CKKS vector square and power functions

7055d17

tests for CKKS vector square and power functions

6612bb9

replace x.mul_inplace(x) with x.square()

1528f0e

zero power

9d2480e

philomath213 added the Type: New Feature ➕ Introduction of a completely new addition to the codebase label Jul 13, 2020

philomath213 requested review from bcebere and youben11 July 13, 2020 16:19

bcebere reviewed Jul 13, 2020

View reviewed changes

requested changes

f01137c

youben11 requested changes Jul 13, 2020

View reviewed changes

add more test-case with power of 4

3314279

philomath213 requested a review from youben11 July 13, 2020 18:22

bcebere approved these changes Jul 13, 2020

View reviewed changes

youben11 reviewed Jul 13, 2020

View reviewed changes

remove _inplace function from python api

27954c9

philomath213 requested a review from youben11 July 14, 2020 05:30

youben11 requested changes Jul 14, 2020

View reviewed changes

philomath213 and others added 5 commits July 14, 2020 10:50

add more test-case with power of 4

0db9caa

fix power_inplace

7865f0e

Co-authored-by: Ayoub Benaissa <ayouben9@gmail.com>

more tests to cover all function branches

6160b6a

more tests

529b647

python linting

846a199

philomath213 requested a review from youben11 July 15, 2020 18:52

youben11 approved these changes Jul 15, 2020

View reviewed changes

youben11 merged commit fdf1add into master Jul 15, 2020

delete-merged-branch bot deleted the ckks-square-power branch July 15, 2020 18:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CKKS square and power functions #100

CKKS square and power functions #100

philomath213 commented Jul 13, 2020 •

edited

Loading

bcebere Jul 13, 2020

philomath213 Jul 13, 2020

bcebere Jul 13, 2020

philomath213 Jul 13, 2020

youben11 Jul 13, 2020

philomath213 Jul 13, 2020

youben11 Jul 13, 2020

philomath213 Jul 13, 2020

youben11 Jul 13, 2020

philomath213 Jul 13, 2020

youben11 Jul 14, 2020

bcebere left a comment

youben11 Jul 13, 2020

youben11 Jul 14, 2020

youben11 Jul 14, 2020

CKKS square and power functions #100

CKKS square and power functions #100

Conversation

philomath213 commented Jul 13, 2020 • edited Loading

Description

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bcebere left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philomath213 commented Jul 13, 2020 •

edited

Loading