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

Updated Keras and TensorFlow Task Runner and related workspaces. #1174

Merged
merged 57 commits into from
Dec 13, 2024

Conversation

tanwarsh
Copy link
Collaborator

@tanwarsh tanwarsh commented Nov 26, 2024

Related issue #973.

Changes in this PR:

  1. Updated Tensorflow and Keras to Tensorflow v2.18.0 and Keras 3
  2. Updated openfl/federated/task/runner_kr.py
  3. Standardizing the workspace.
  4. Removed workspaces.
  5. Updated workspaces.
    1. keras_cnn_mnist
    2. keras_nlp
    3. keras_2dunet

@tanwarsh tanwarsh changed the title Updated Keras and TensorFlow. WIP:Updated Keras and TensorFlow. Nov 26, 2024
@tanwarsh tanwarsh marked this pull request as draft November 26, 2024 13:08
@tanwarsh tanwarsh marked this pull request as ready for review November 27, 2024 06:01
@tanwarsh tanwarsh changed the title WIP:Updated Keras and TensorFlow. Updated Keras and TensorFlow. Nov 27, 2024
@tanwarsh
Copy link
Collaborator Author

I will add code changes for the remaining Keras workspace - Keras_cnn_with_compression and keras_nlp.

@teoparvanov @kta-intel @psfoley, I was facing an issue with optimizer weights due to resetting the weights before saving them to the tensor database and using them to rebuild the model, which resulted in no improvement in accuracy. I have skipped the rebuild (and hence resetting the model optimizer weights) for the first round. This can be further discussed, and we can proceed with these changes once all the pipelines are working and the workspace changes are also completed.

@tanwarsh tanwarsh dismissed psfoley’s stale review December 11, 2024 08:57

requested changes are done.

Copy link
Collaborator

@theakshaypant theakshaypant left a comment

Choose a reason for hiding this comment

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

Is openfl setup not affected by these changes?

openfl/federated/task/runner_keras_tf.py Outdated Show resolved Hide resolved
openfl-workspace/keras_nlp/src/dataloader.py Outdated Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

A reader cannot tell what this example is about (model/use case). Suggest adding a header (also making the license header consistent with other files) that tells what architecture, and what use case this example is for.

openfl/federated/__init__.py Outdated Show resolved Hide resolved
@MasterSkepticista
Copy link
Collaborator

I think this is a good place to refactor the directory structure:

openfl-workspace (in future, "examples")
 |_ keras
 |  |_ mnist
 |  |_ lstm
 |_ (other backends in future PRs)

Thoughts?

@teoparvanov
Copy link
Collaborator

I think this is a good place to refactor the directory structure:

openfl-workspace (in future, "examples")
 |_ keras
 |  |_ mnist
 |  |_ lstm
 |_ (other backends in future PRs)

Thoughts?

@MasterSkepticista , would it be acceptable to do this in a follow-up PR?

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.

Great work, thanks @tanwarsh ! This PR brings OpenFL back into the TensorFlow game 😉

openfl/federated/task/runner_keras_tf.py Outdated Show resolved Hide resolved
@tanwarsh
Copy link
Collaborator Author

Is openfl setup not affected by these changes?

No this will not affect openfl setup.

@tanwarsh tanwarsh marked this pull request as draft December 12, 2024 15:14
Signed-off-by: yes <[email protected]>
Signed-off-by: yes <[email protected]>
@tanwarsh tanwarsh marked this pull request as ready for review December 13, 2024 07:12
Signed-off-by: yes <[email protected]>
@tanwarsh
Copy link
Collaborator Author

@teoparvanov @MasterSkepticista @kta-intel

As discussed I have migrated workspaces mentioned below to latest tensorflow and keras 3 versions and rest of the workspaces are removed.

  1. keras_cnn_mnist
  2. keras_nlp
  3. keras_2dunet (renamed from tf_2dunet)

@tanwarsh tanwarsh changed the title Updated Keras and TensorFlow. Updated Keras and TensorFlow Task Runner and related workspaces. Dec 13, 2024
@teoparvanov teoparvanov mentioned this pull request Dec 13, 2024
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.

LGTM!

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.

8 participants