Conversation
|
|
xieyxclack
left a comment
xieyxclack
left a comment
There was a problem hiding this comment.
- Maybe we can merge the
caesar_v_flandvertical_fltogether, so that some functionalities can be reused, such aspaillier,dataloader, andutils.py. - Please resolve the conflicts in
federatedscope/core/auxiliaries/data_builder.py - A unittest is required for evaluating the correctness.
- Some minor suggestions are listed inline, and I would review the details of server/client later
| @@ -0,0 +1,47 @@ | |||
| # You can refer to pyphe for the detail implementation. ( | |||
There was a problem hiding this comment.
This file is the same as federatedscope/vertical_fl/Paillier/abstract_paillier.py, maybe you can reuse it.
federatedscope/caesar_v_fl/README.md
Outdated
| @@ -0,0 +1,13 @@ | |||
| ### Caesar Vertical Federated Learning | |||
|
|
|||
| We provide an example for seCure lArge-scalE SlArse logistic Regression (caesar) vertical federated learning, you can run with: | |||
There was a problem hiding this comment.
Add the references here.
…to dev_caesar_vfl
…to dev_caesar_vfl
xieyxclack
left a comment
xieyxclack
left a comment
There was a problem hiding this comment.
LGTM, please see the inline comments, thanks!
| self.maximum = 2**size | ||
| self.mod_number = 2 * self.maximum + 1 | ||
| self.epsilon = 1e8 | ||
| self.epsilon = 1e4 |
There was a problem hiding this comment.
Is 1e-4 enough for ensuring precise results?
There was a problem hiding this comment.
yes, setting 1e-8 will not be better, and sometimes worse depending on the size below
| recover it by summing up | ||
| """ | ||
| def __init__(self, shared_party_num, size=60): | ||
| def __init__(self, shared_party_num, size=20): |
There was a problem hiding this comment.
IMO, setting size=20 is not secure here
There was a problem hiding this comment.
how about 50? I tried epsilon=1e4 and size=50, and the acc is 0.82. Making size larger, the acc will decrease.
There was a problem hiding this comment.
I am not sure why the acc would be affected by the size
| X[:, j] = 0 | ||
| return X | ||
|
|
||
| def normalize(X): |
| data[1]['val'] = None | ||
| data[1]['test'] = test_data | ||
|
|
||
| # For Client #2 |
There was a problem hiding this comment.
So we assume that client_2 owns the labels? maybe we can add some annotations here.
| @@ -0,0 +1,18 @@ | |||
| ### Caesar Vertical Federated Learning | |||
There was a problem hiding this comment.
Maybe we can move this README.md to /federatedscope/vertical/ and merge it with that of secure_LR
|
|
||
| return secret_seq | ||
|
|
||
| def secret_split_for_piece_of_ss(self, secret): |
There was a problem hiding this comment.
So the differences between secret_sharing and simple_secret_sharing is the function secret_split and secret_split_for_piece_of_ss? So maybe the class AdditiveSecretSharing in simple_secret_sharing can be inherited from that of secret_sharing. Or just add a config (e.g., vertical.use_for_pieceof_ss)?
| if not self.own_label: | ||
| self.a_computes() | ||
|
|
||
| # A computes <z>_1 = <z_a>_1 + <<z_a>_2>_1 + <<z_b>_1>_1, |
There was a problem hiding this comment.
We can use client_with_label/client_without_label rather than A/ B to make this more readable.
3 participants
an example for seCure lArge-scalE SpArse logistic Regression (caesar) vertical federated learning