-
Notifications
You must be signed in to change notification settings - Fork 181
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 TensorFlow client api example #1982
Add TensorFlow client api example #1982
Conversation
43780ee
to
07bbaab
Compare
/build |
examples/hello-world/ml-to-fl/jobs/client_api/app/config/config_exchange.conf
Outdated
Show resolved
Hide resolved
examples/hello-world/ml-to-fl/jobs/tensorflow/app/custom/tf_model_persistor.py
Outdated
Show resolved
Hide resolved
examples/hello-world/ml-to-fl/jobs/tensorflow/app/custom/tf_utils.py
Outdated
Show resolved
Hide resolved
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.
add few comments
33849e3
to
4f86ca9
Compare
examples/hello-world/ml-to-fl/pt/job_templates/client_api/config_exchange.conf
Outdated
Show resolved
Hide resolved
examples/hello-world/ml-to-fl/pt/job_templates/client_api/config_exchange.conf
Outdated
Show resolved
Hide resolved
examples/hello-world/ml-to-fl/pt/job_templates/client_api/config_exchange.conf
Outdated
Show resolved
Hide resolved
examples/hello-world/ml-to-fl/pt/job_templates/client_api/config_exchange.conf
Outdated
Show resolved
Hide resolved
examples/hello-world/ml-to-fl/pt/job_templates/client_api/config_exchange.conf
Outdated
Show resolved
Hide resolved
examples/hello-world/ml-to-fl/pt/job_templates/client_api/info.conf
Outdated
Show resolved
Hide resolved
examples/hello-world/ml-to-fl/pt/job_templates/client_api/info.md
Outdated
Show resolved
Hide resolved
examples/hello-world/ml-to-fl/pt/job_templates/decorator/config_exchange.conf
Outdated
Show resolved
Hide resolved
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.
add few more comments
9c8b10f
to
b880e43
Compare
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.
In general looks Good.
please double check the followings before merge
- since you removed the config_exchange.conf, can you make sure the job CLI still works ?
- The job CLI tutorial shows the config_exchange.conf, can you check the notebooks and remove the config_exchange related command
- step and step examples notebooks also has markup and notes referring config_exchange.conf please correct them as needed.
- fix the 3.9 unit test failure
b880e43
to
672df0b
Compare
/build |
Jenkins not working because of changes in #1984 Yan is working on a fix for that, we can ignore the blossom-ci tests for now. |
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
* Rename script names to avoid confusion * delete sag_lightning job templates * Add more comments to job configurations * Rename folder codes to pt * Return model copy but not the cache model directly * Fix paths in step-by-step * Add TensorFlow example * Fix meta and link * Use job_templates to avoid duplicate codes * Change wording * Fix link * Use tensorflow save_weights/load_weights method * Fix wording * Use two job_templates * Combine config_exchange and config_fed_client * Address comments * Remove config exchange and fix unit test * Remove invalid test entry
Description
Types of changes
./runtest.sh
.