Skip to content
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 protect method for feature order in fl-xgb #497

Merged
merged 24 commits into from
Jan 31, 2023

Conversation

xieyxclack
Copy link
Collaborator

@xieyxclack xieyxclack commented Jan 17, 2023

Thanks a lot for the co-contributor @qbc2016, and sorry the mess commits due to the squash merge of #482

@xieyxclack xieyxclack added Feature New feature Tree labels Jan 17, 2023
@xieyxclack xieyxclack requested a review from qbc2016 January 17, 2023 12:44
min_value = min(data[feature_order_frame][:, i])
max_value = max(data[feature_order_frame][:, i])
if j == 0:
self.split_value[i][idx_end] = max_value
Copy link
Collaborator

Choose a reason for hiding this comment

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

The value on the right hand side should be divided by 2.0

'extra_info': extra_info
}

def _get_feature_order_info(self, data):
Copy link
Collaborator

Choose a reason for hiding this comment

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

For convenience, I also protected the label owner's feature order before. Actually, label owner does not need to do this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since it need some more efforts to fix this issue such as modifying the split position accordingly, we can add TODO item here and fix it later

@xieyxclack xieyxclack changed the title [WIP] Add protect method for feature order in fl-xgb Add protect method for feature order in fl-xgb Jan 30, 2023
Copy link
Collaborator

@qbc2016 qbc2016 left a comment

Choose a reason for hiding this comment

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

Good job!

@xieyxclack xieyxclack merged commit 8d042b2 into alibaba:master Jan 31, 2023
This was referenced Jan 31, 2023
@xieyxclack xieyxclack deleted the dev/xgb-use-bins branch April 3, 2023 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature Tree
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants