-
Notifications
You must be signed in to change notification settings - Fork 214
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
Resolve Visibility of Aggregator Private Attributes in Experimental Workflow #1084
Resolve Visibility of Aggregator Private Attributes in Experimental Workflow #1084
Conversation
…enhanced tcs Signed-off-by: refai06 <[email protected]>
…testcase enhanced Signed-off-by: refai06 <[email protected]>
Signed-off-by: refai06 <[email protected]>
Signed-off-by: refai06 <[email protected]>
Signed-off-by: refai06 <[email protected]>
self.execute_task_args = ( | ||
self, | ||
f, | ||
parent_func, | ||
FLSpec._clones, | ||
agg_to_collab_ss, | ||
kwargs, | ||
) |
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.
If we remove the private attribute filtering from here, then who is responsible for doing it, especially if we execute the FLSpec via the run()
method, rather than through the export feature?
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.
Private attribute filtering is required while transitioning from aggregator to collab (and vice-versa). For LocalRuntime
it is done inside Participants (openfl/experimental/interface/participants.py) and in FederatedRuntime
it is done inside Component (openfl/experimental/component/aggregator/aggregator.py and openfl/experimental/component/collaborator/collaborator.py)
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.
I see, thanks for the explanation, @refai06 - the distinction is indeed not obvious for the "uninitiated" reader...
As a follow-up PR, I'd suggest re-organizing a little bit the code, to make it easier to navigate and review. Right now, everything related to Workflow API resides directly under openfl/experimental
- thus implicitly "disallowing" other experimental features. Instead, it may be better to have a dedicated openfl/experimental/workflow
package for example. Additionally, under openfl/experimental/workflow
, we could imagine separate sub-packages for local
, federated
and export
for example.
WDYT?
…ipant refactor Signed-off-by: refai06 <[email protected]>
Signed-off-by: refai06 <[email protected]>
...stcase_private_attributes_initialization_with_both_options/src/testflow_privateattributes.py
Outdated
Show resolved
Hide resolved
...stcase_private_attributes_initialization_with_both_options/src/testflow_privateattributes.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.
A general question regarding test coverage - do the test cases under tests/github/experimental/workspace
cover both "runtimes":
- the local runtime Aggregator and Collaborator classes in
participants.py
- the federated runtime Aggregator and Collaborator classes from
experimental.component
?
Signed-off-by: refai06 <[email protected]>
@teoparvanov : |
Background
This PR solves the (Experimental Workflow Interface) Aggregator private attributes visible to Collaborators #1052
Change Description
FLSpec
classdo_task
method to handle thefilter_exclude_include
functionality during the aggregator to collaborator transition.filter_exclude_include
functionality from the next method of theFLSpec
classModifications
openfl/experimental/component/aggregator/aggregator.py
openfl/experimental/interface/fl_spec.py
tests/github/experimental/workspace/testcase_private_attributes/src/aggregator_private_attrs.py
tests/github/experimental/workspace/testcase_private_attributes/src/testflow_privateattributes.py
tests/github/experimental/workspace/testcase_private_attributes_initialization_with_both_options/src/aggregator_private_attrs.py
tests/github/experimental/workspace/testcase_private_attributes_initialization_with_both_options/src/testflow_privateattributes.py
tests/github/experimental/workspace/testcase_private_attributes_initialization_without_callable/src/aggregator_private_attrs.py
tests/github/experimental/workspace/testcase_private_attributes_initialization_without_callable/src/testflow_privateattributes.py
Verification
Verified all testcases from tests/github/experimental/workspace
-tests/github/experimental/workspace/testcase_datastore_cli
-tests/github/experimental/workspace/testcase_include_exclude
-tests/github/experimental/workspace/testcase_private_attributes
-tests/github/experimental/workspace/testcase_private_attributes_initialization_with_both_options
-tests/github/experimental/workspace/testcase_private_attributes_initialization_without_callable
-tests/github/experimental/workspace/testcase_internalloop
-tests/github/experimental/workspace/testcase_reference
-tests/github/experimental/workspace/testcase_reference_with_include_exclude
-tests/github/experimental/workspace/testcase_subset_of_collaborators
Tutorials
Openfl-workspace/experimental/
-101_torch_cnn_mnist
-301_torch_cnn_mnist_watermarking