-
Notifications
You must be signed in to change notification settings - Fork 935
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 FL+XGBoost Baseline #2226
Add FL+XGBoost Baseline #2226
Conversation
Hi @jafermarq That's the first working version of the centralized baseline, those are the results that I got for now:
Results for the centralized model after shuffling the dataset instead of choosing the first 75% rows of the dataset:
I use the same metrics as the ones in the paper -Accuracy for binary classification, and mean_squared_error for regression-, I tried different numbers of estimators: (100,500), The ones that I'm showing are for The results are kinda different from the ones shared on the paper:
As you can see the main differences are in cod-rna and abalone datasets. If you can give me any tips that can help me with that, please do. If you have any notes on the implementation style so far, please let me know so I can modify them. I'll go through the paper and the notebook you provided again to see if I missed any details and also, and I'll also try and reach out to one of the authors to see if he can help with that. |
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.
I made some changes to the code:
- removed top-level
.gitignore
changes and moved them to a.gitignore
local to your baseline. - minor change to
__init__.py
- moved
print
inmain.py
afterload_single_dataset()
into that function. (<-- could you double check if this looks good to you?) - Specified types in missing places (e.g. in
client.py
)
just a couple of small request:
- could you describe either in the
README.md
or at the top of yourserver.py
why you need a custom server class? (I know why, but mentioning this would be useful for others out there). Keeping it brief ~100word is sufficient. - Would it make sense to move your
sweep.yaml
insideconf/
?
Hi @jafermarq,
|
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.
@Aml-Hassan-Abd-El-hamid, this looks good! We'll merge it very soon. 💯
Closes Issue #2220
Checklist
(I'll be adding more details later as I go)
Centralized model:
FedXGBllr:
2 clients:
5 clients:
10 clients:
Testing
Documentation