-
Notifications
You must be signed in to change notification settings - Fork 212
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
Add xgb homoeval #549
Add xgb homoeval #549
Conversation
* add homo evaluation and add AUC to binary loss
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
God job! Please see the inline comments for minor suggestions, thanks a lot
@@ -95,6 +95,7 @@ def extend_fl_setting_cfg(cfg): | |||
# Default values for 'dp': {'bucket_num':100, 'epsilon':None} | |||
# Default values for 'op_boost': {'algo':'global', 'lower_bound':1, | |||
# 'upper_bound':100, 'epsilon':2} | |||
cfg.vertical.eval = '' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add comments
@@ -27,9 +27,11 @@ def _process_y_pred(self, y_pred): | |||
|
|||
def get_metric(self, y, y_pred): | |||
y_pred = self._process_y_pred(y_pred) | |||
from sklearn import metrics |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move to top of this file
federatedscope/vertical_fl/utils.py
Outdated
@@ -1,6 +1,8 @@ | |||
from federatedscope.vertical_fl.xgb_base.worker import wrap_client_for_train, \ | |||
wrap_server_for_train, wrap_client_for_evaluation, \ | |||
wrap_server_for_evaluation | |||
from federatedscope.vertical_fl.xgb_base.worker.homo_evaluation_wrapper\ | |||
import wrap_client_for_homo_evaluation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
homo
- > he
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please change the name of this file
@@ -30,6 +30,14 @@ def __init__(self, | |||
self.msg_buffer = {'train': {}, 'eval': {}} | |||
self.client_num = self._cfg.federate.client_num | |||
|
|||
# TODO: the following is used for homo_evaluation, | |||
# which may be put in an if condition | |||
from federatedscope.vertical_fl.Paillier import \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please move this to the top of this file
|
||
__all__ = [ | ||
'XGBServer', 'XGBClient', 'wrap_server_for_train', 'wrap_client_for_train', | ||
'wrap_server_for_evaluation', 'wrap_client_for_evaluation' | ||
'wrap_server_for_evaluation', 'wrap_client_for_evaluation', | ||
'wrap_client_for_homo_evaluation' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
cfg.vertical.dims = [7, 14] | ||
cfg.vertical.algo = 'xgb' | ||
cfg.vertical.data_size_for_debug = 2000 | ||
cfg.vertical.eval = 'homo' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change homo
@@ -0,0 +1,326 @@ | |||
import types |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: shall we merge this file with another evaluation wrapper?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
-) add AUC to binary loss
-) add homo evaluation