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 pytorch MNIST Workflow tutorial with FedProx #1158

Conversation

ynonflumintel
Copy link
Contributor

@ynonflumintel ynonflumintel commented Nov 19, 2024

This branch introduces a tutorial for setting up a neural network training federation using pytorch, the MNIST dataset using the openfl workflow API.

@teoparvanov teoparvanov changed the title Add pytorch MNIST Workflow tutorial Add pytorch MNIST Workflow tutorial with FedProx Nov 19, 2024
@rahulga1
Copy link
Collaborator

@ynonflumintel I think this is not the correct file in this commit, I dont see workflow api getting used in this file.
can you please confirm?

@ynonflumintel
Copy link
Contributor Author

@ynonflumintel I think this is not the correct file in this commit, I dont see workflow api getting used in this file. can you please confirm?

I pushed the wrong version of the file before, please take a look now

Copy link
Collaborator

@teoparvanov teoparvanov left a comment

Choose a reason for hiding this comment

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

Looks great, thanks @ynonflumintel ! I just have a couple of small suggestions:

Copy link
Collaborator

@teoparvanov teoparvanov left a comment

Choose a reason for hiding this comment

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

PS: don't forget to sign off your commits for a successful CI run (DCO job):

git commit -s -m "..."

Copy link
Collaborator

@MasterSkepticista MasterSkepticista left a comment

Choose a reason for hiding this comment

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

General comment on the entire notebook: Include output of all the cells. New users should have visibility into the cells and evaluate for themselves if the notebook fulfils their goal of visit. Not showing output, can put-off newcomers because they end goal is not "visible".

" loss.backward()\n",
" self.optimizer.step()\n",
"\n",
" if (len(data) * batch_idx) / len(self.train_loader.dataset) >= log_threshold:\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of this complicated logic of calculating "quarter" of samples used, it is OK to print every 10 or 100 steps. We need to be cautious of all the cognitive overhead we put on the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ran experiments where there were a couple of thousand steps (small chunks), it's too spammy IMO

@ynonflumintel
Copy link
Contributor Author

General comment on the entire notebook: Include output of all the cells. New users should have visibility into the cells and evaluate for themselves if the notebook fulfils their goal of visit. Not showing output, can put-off newcomers because they end goal is not "visible".

Thought it would be too verbose and increase the file size significantly, but I added them as per your request

@ynonflumintel ynonflumintel force-pushed the yf/add-pytorch-workflow-example branch 3 times, most recently from b8fbfbc to 30ecf9e Compare November 20, 2024 13:18
@teoparvanov teoparvanov merged commit 918e125 into securefederatedai:develop Nov 20, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants