-
Notifications
You must be signed in to change notification settings - Fork 235
Hotstaging area #1474
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
Hotstaging area #1474
Changes from all commits
f756c8b
fb0f7ef
0265e63
c621b41
3cddda8
1d48e3e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -364,16 +364,11 @@ def get_tasks(self): | |
| tasks[task]["aggregation_type"] = aggregation_type | ||
| return tasks | ||
|
|
||
| def get_aggregator(self, tensor_dict=None): | ||
| def get_aggregator(self): | ||
| """Get federation aggregator. | ||
|
|
||
| This method retrieves the federation aggregator. If the aggregator | ||
| does not exist, it is built using the configuration settings and the | ||
| provided tensor dictionary. | ||
|
|
||
| Args: | ||
| tensor_dict (dict, optional): The initial tensor dictionary to use | ||
| when building the aggregator. Defaults to None. | ||
| does not exist, it is built using the configuration settings. | ||
|
|
||
| Returns: | ||
| self.aggregator_ (Aggregator): The federation aggregator. | ||
|
|
@@ -401,7 +396,7 @@ def get_aggregator(self, tensor_dict=None): | |
| # TODO: Load callbacks from plan. | ||
|
|
||
| if self.aggregator_ is None: | ||
| self.aggregator_ = Plan.build(**defaults, initial_tensor_dict=tensor_dict) | ||
| self.aggregator_ = Plan.build(**defaults) | ||
|
Comment on lines
-404
to
+399
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remnant of interactive API |
||
|
|
||
| return self.aggregator_ | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -175,27 +175,7 @@ def GetTasks(self, request, context): # NOQA:N802 | |
| tasks, round_number, sleep_time, time_to_quit = self.aggregator.get_tasks( | ||
| request.header.sender | ||
| ) | ||
| if tasks: | ||
| if isinstance(tasks[0], str): | ||
| # backward compatibility | ||
| tasks_proto = [ | ||
| aggregator_pb2.Task( | ||
| name=task, | ||
| ) | ||
| for task in tasks | ||
| ] | ||
| else: | ||
| tasks_proto = [ | ||
| aggregator_pb2.Task( | ||
| name=task.name, | ||
| function_name=task.function_name, | ||
| task_type=task.task_type, | ||
| apply_local=task.apply_local, | ||
| ) | ||
| for task in tasks | ||
| ] | ||
| else: | ||
| tasks_proto = [] | ||
| tasks_proto = [aggregator_pb2.Task(name=task) for task in tasks] | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. these 20 lines are same as the one line, and covers all edge cases (note that removal of internal if-else is partly because interactive API is defunct) |
||
|
|
||
| header = create_header( | ||
| sender=self.aggregator.uuid, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -123,8 +123,8 @@ def test_time_to_quit(agg, round_number, rounds_to_train, expected): | |
|
|
||
| @pytest.mark.parametrize( | ||
| 'col_name,tasks,time_to_quit,exp_tasks,exp_sleep_time,exp_time_to_quit', [ | ||
| ('col1', ['task_name'], True, None, 0, True), | ||
| ('col1', [], False, None, 10, False), | ||
| ('col1', ['task_name'], True, [], 0, True), | ||
| ('col1', [], False, [], 10, False), | ||
|
Comment on lines
-126
to
+127
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tasks are now returned as lists. It is strange to expect a Now this is consistent across the entire component. |
||
| ('col1', ['task_name'], False, ['task_name'], 0, False), | ||
| ]) | ||
| def test_get_tasks(agg, col_name, tasks, time_to_quit, | ||
|
|
||
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.
Remnant of interactive API