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

Update simulation examples to use flwr_datasets #2381

Merged
merged 27 commits into from
Dec 7, 2023

Conversation

jafermarq
Copy link
Contributor

@jafermarq jafermarq commented Sep 16, 2023

This PR updates the simulation-pytorch and simulation-tensorflow examples. They now use the Flower datasets API. The examples run just fine. Some things to iron out before merging:

  • Update URLs once datasets docs are live
  • Update pyproject.toml once flwr_datasets is in PyPI
  • Can we disable tqdm ?
  • Reflect this PR in change log.
  • Once happy with both TF and Pytorch sim.py --> apply changes to notebook

@jafermarq jafermarq changed the title Simulation examples with flower datasets Update simulation examples to use flwr_datasets Sep 16, 2023
@jafermarq jafermarq added the part: examples Add or update a Flower example label Sep 20, 2023
Copy link
Contributor

@adam-narozniak adam-narozniak left a comment

Choose a reason for hiding this comment

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

The jupyter notebooks files need to be updated.

For the tensorflow, I think it's better if we use this approach (instead of directly to tensors)

tf_dataset = partition.to_tf_dataset(columns="img", label_cols="label", batch_size=64,
shuffle=True) # model.fit has shuffle = True so that is also not 100% needed

Or alternatively .with_format("numpy").

One thing about the .with_format("tf") I'm not sure is what happens if the GPU is present (tf by default uses GPU if it's available, so I'm not sure if the full data goes to GPU when e.g. trainset['images'] because it fetches all the images and it happens later in the code)

@jafermarq jafermarq marked this pull request as ready for review November 23, 2023 18:41
adam-narozniak
adam-narozniak previously approved these changes Nov 24, 2023
Copy link
Contributor

@adam-narozniak adam-narozniak left a comment

Choose a reason for hiding this comment

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

LGTM! I like the change in the TF example, it looks clean.

@danieljanes danieljanes enabled auto-merge (squash) November 28, 2023 16:07
danieljanes
danieljanes previously approved these changes Dec 7, 2023
@danieljanes danieljanes merged commit 7d18323 into main Dec 7, 2023
@danieljanes danieljanes deleted the simulation_examples_with_flower_datasets branch December 7, 2023 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
part: examples Add or update a Flower example
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants