-
Notifications
You must be signed in to change notification settings - Fork 214
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
Update xgb algo files #439
Conversation
qbc2016
commented
Nov 21, 2022
- refine each client's test data
- refine testing procedure
- add 3 more datasets for xgb algo
…bel form 0 to -1 or not
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, please refer to inline comments for more suggestions
|
||
Arguments: | ||
root (str): root path | ||
name (str): name of dataset, ‘adult’ or ‘xxx’ |
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.
adult?
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.
Ah, sry.., it should be 'blog'
|
||
Arguments: | ||
root (str): root path | ||
name (str): name of dataset, ‘adult’ or ‘xxx’ |
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
@@ -0,0 +1,155 @@ | |||
import logging |
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.
It seems that we can define a BaseDataset
for vertical dataset
} | ||
if tree_num + 1 < self.client.num_of_trees: | ||
# TODO: add feedback during training | ||
''' |
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 remove this if it is redundant
|
||
tree_num = 0 | ||
self.test_for_root(tree_num) | ||
def evaluation(self): |
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.
Can Test_base
support multiple (>2) clients?
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.
Yes.
if self.own_label: | ||
self.test_y = self.data['test']['y'] | ||
|
||
self.test_z = np.zeros(self.test_x.shape[0]) |
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.
what test_z
means?
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.
It is exactly the result outputted by the model on test data, I'll change the name.
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.
The dataset part looks good to me. As the XGB module is independent, please consider adding it to SPHINX.
@@ -1,6 +1,10 @@ | |||
import numpy as np |
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.
feature_partition=feature_partition, | ||
tr_frac=splits[0], | ||
download=True, | ||
seed=1234, |
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.
We might maintain a global seed for data generation as it has been set in here. @xieyxclack @yxdyc @DavdGao