-
Notifications
You must be signed in to change notification settings - Fork 875
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
refactor: use alexnet to replace efficientnet #2782
Conversation
Hi @helin0815, I think instead of removing |
@jafermarq good.The problem I am currently facing is that if using Efficientnet, adding a little bit of noise during differential privacy noise addition can cause loss to become nan. When I try using Alexnet, I can add noise normally and I will add this part to Readme |
@jafermarq hello,please review again. |
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.
Hi @helin0815,
I updated your branch with the latest main
. I had to fix some merge conflicts because we recently updated the advanced-pytorch
example. I also did some small modifications to how --model
used.
Currently your example with --model alexnet
doesn't work because the CIFAR-10 batches are of shape 224x224
but your model expects them to be 32x32
... I'll get back to you about this early in the week. maybe it's a good idea to update the example to user 32x32
.
In the mean time, could you update the README and show how people using the example can change the model?
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
@jafermarq hello bro,I just deleted the differential privacy related section in readme. What should I do next |
…el choices in readme; formatting
I pushed with some changes. The main one is replacing I added a couple of sentences at the end of the reamde to indicate how people can switch between efficientnet/alexent. Please take a look. Something else to include? |
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.
@helin0815 , Thanks for the additions to this example. This will be useful for users that want to see how easy it is to switch between ML models for Flower clients. 💯
@jafermarq Thank you for submitting the code and modifying the readme so that this PR can be merged smoothly. When you last replied to me, I was already asleep. In China, this is my sleeping time. I hope this PR can be helpful to other flower users. Thank you for your efforts |
Issue
Description
Proposal
Explanation
Changelog entry
Any other comments?