[Workflow Interface] Refactor FLSpec and Runtime to enhance modularity #1363
Conversation
Signed-off-by: Ishant Thakare <[email protected]>
Signed-off-by: Ishant Thakare <[email protected]>
Signed-off-by: Ishant Thakare <[email protected]>
Signed-off-by: Ishant Thakare <[email protected]>
Signed-off-by: Ishant Thakare <[email protected]>
Signed-off-by: Ishant Thakare <[email protected]>
Signed-off-by: Ishant Thakare <[email protected]>
Signed-off-by: Ishant Thakare <[email protected]>
Signed-off-by: Ishant Thakare <[email protected]>
Signed-off-by: Ishant Thakare <[email protected]>
Signed-off-by: Ishant Thakare <[email protected]>
Signed-off-by: Ishant Thakare <[email protected]>
Signed-off-by: Ishant Thakare <[email protected]>
Signed-off-by: Ishant Thakare <[email protected]>
Signed-off-by: Ishant Thakare <[email protected]>
ishant162
requested review from
rajithkrishnegowda,
ishaileshpant,
tanwarsh and
teoparvanov
as code owners
...ntal/workflow/FederatedRuntime/testcase_datastore_cli/workspace/testflow_datastore_cli.ipynb
Show resolved
Hide resolved
.../experimental/workflow/FederatedRuntime/101_MNIST/workspace/101_MNIST_FederatedRuntime.ipynb
Show resolved
Hide resolved
Signed-off-by: Ishant Thakare <[email protected]>
There was a problem hiding this comment.
Looks good now, thanks for addressing my comments, @ishant162 !
|
Hi @theakshaypant, The following pipeline has been tested locally with the changes and should work once the PR is committed:
The following pipeline requires rework to adapt test cases and will be addressed in a follow-up PR:
I am currently debugging the following pipeline failure, which appears to be a timeout issue:
|
There was a problem hiding this comment.
Thanks @ishant162 - I've reviewed the PR. No issue with the code implementation, but we need to discuss further if there is a way to get updated information about the collaborator list with the API change proposed. We can discuss in our meeting on 3/3.
| aggregator begins the flow with :code:`start` task and optionally passed in model and optimizer. The list of collaborators in the federation, :code:`self.collaborators`, | ||
| is automatically populated by the Runtime infrastructure. It serves as the participant list for executing tasks listed in :code:`self.next` and :code:`aggregated_model_validation`. |
There was a problem hiding this comment.
Looks like this add a reserved keyword collaborators to the flow spec. I assume this is set statically at the start of the workflow execution? Is there any way to get an updated collaborator from inside the workload? Use case - for the FederatedRuntime if additional envoys registered with the director, how could they join the federation after experiment start (assume TLS is setup and handled)? With the current API (self.collaborators = self.runtime.collaborators), the flow could (in a future release) get access to this list by querying the runtime. This is a capability of the RANO fork of OpenFL used in the real world; where not all collaborators will necessarily be ready (or known) at the start of an experiment.
There was a problem hiding this comment.
Would also suggest making the required changes that are made in #1421 for this refactor.
Signed-off-by: Ishant Thakare <[email protected]>
|
This was discussed further offline. This change will result in limiting the ability to add participants dynamically in the future. This PR should not be merged until the implications are well understood. Aim to reach a decision on this by Friday 3/7. |
@theakshaypant : Changes made in #1421 are not related to refactoring of either the FLSpec or Runtime. Therefore they should NOT be included in this PR. Please see the updated disposition in #1421 |
|
This PR should be kept on hold due to a blocking issue found while updating tutorials for a follow-up PR. Investigation is ongoing, with an update expected early next week. |
Background
Proposal: #1317
Change Description
FLSpec,LocalRuntime,FederatedRuntimeandAggregatorclasses.run()method implementation fromFLSpectoRuntime.FLSpec, enabling foreach operations, necessitating changes to the execution flow.WorkspaceExportfunctionality to align with the refactor changes.Verification
tests/github/experimental/workflow/LocalRuntime.tests/github/experimental/workflow/FederatedRuntime.101_MNISTofLocalRuntimeand101_MNIST,301_MNIST_WatermarkingofFederatedRuntime.File Modifications
docs/about/features_index/workflowinterface.rstopenfl-tutorials/experimental/workflow/101_MNIST.ipynbopenfl-tutorials/experimental/workflow/FederatedRuntime/101_MNIST/workspace/101_MNIST_FederatedRuntime.ipynbopenfl-tutorials/experimental/workflow/FederatedRuntime/301_MNIST_Watermarking/workspace/MNIST_Watermarking.ipynbopenfl/experimental/workflow/component/aggregator/aggregator.pyopenfl/experimental/workflow/interface/fl_spec.pyopenfl/experimental/workflow/runtime/federated_runtime.pyopenfl/experimental/workflow/runtime/local_runtime.pyopenfl/experimental/workflow/utilities/runtime_utils.pyopenfl/experimental/workflow/workspace_export/export.pytests/github/experimental/workflow/LocalRuntime.tests/github/experimental/workflow/FederatedRuntime.Changelog:
Known Issues
Note:
Following changes will be done in a follow-up PR:
LocalRuntime&FederatedRuntimeTutorials.