-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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
Implement secure boost scheme - secure evaluation and validation (during training) without local feature leakage #10079
Implement secure boost scheme - secure evaluation and validation (during training) without local feature leakage #10079
Conversation
…ute under secure scenario
…valent to broadcast
…lobal best split, but need to further apply split correctly
…ute under secure scenario
…valent to broadcast
…lobal best split, but need to further apply split correctly
Add alternate vertical splits
…x for training phase
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 code looks good to me overall. We can merge it once we have some basic unittests.
As for integration tests in Python with nvflare (in future PRs), we can assert that
- models are different for different workers.
- predictions are the same
- evaluation result are the same
- only works if the 0th worker has the label.
I highly recommend using the hypothesis test framework (see python tests in xgboost and search the term hypothesis).
Thanks! @YuanTingHsieh , could you add the unit tests according to @trivialfis 's suggestions? |
Those points are all for integration tests not for small unittest. I think the integration tests in Python with nvflare will take more effort, we don't need to rush it in this PR. |
Hi, is there any update? |
Thanks for asking! :) @YuanTingHsieh has been busy with a related NVFlare release in the past two weeks, now the release is close to finish, he will have time to work on this soon. |
Add secure inf unit tests
@trivialfis Yuanting just added some unit tests, seems there is a failed R-test, but not sure if it is related to our modifications, the error message being
|
That should be unrelated, will look into this PR today. |
Hi @trivialfis , thanks for the updates, just merged it. |
Triggered the rest of the CI. |
Hi @trivialfis , there are 3 failed checks, but I think they align with the rebase merge, shall we just merge this? Thanks! |
Please fix errors on buildkite. |
The error is But I do not think it is part of this PR? |
And the other two errors are
I don't quite get what they indicate. |
Let's ignore the Windows error for now and get the Linux buildkite ci to pass: https://buildkite.com/xgboost/xgboost-ci/builds/5116#018f2a11-c525-41a1-a9d7-81bfdb1f701c |
Thanks! addressed the warning in clang-tidy |
Excellent! Please continue to fix other errors in the Linux CI (at the moment, it's the memory leak reported by the leak sanitizer). |
Linux buildkite ci passed with LeakSanitizer fix :) |
8585df5
into
dmlc:vertical-federated-learning
… (#10530) --------- Co-authored-by: Ziyue Xu <[email protected]>
…10079) (dmlc#10530) --------- Co-authored-by: Ziyue Xu <[email protected]>
For implementing Vertical Federated Learning with Secure Features, as discussed in
#9987
This part is independent from the encryption and the alternative vertical pipeline. The purpose is to avoid leaking the real cut value information from participants. Hence add as a separate PR.
This PR is based on #10037, which should be reviewed and merged first.