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

[feature] samplers #200

Merged
merged 3 commits into from
Jul 6, 2022
Merged

[feature] samplers #200

merged 3 commits into from
Jul 6, 2022

Conversation

xieyxclack
Copy link
Collaborator

The sampler is used to sample clients from the idle clients at each training round

@xieyxclack xieyxclack added the Feature New feature label Jul 5, 2022
@xieyxclack xieyxclack requested a review from joneswong July 5, 2022 06:22
@xieyxclack xieyxclack self-assigned this Jul 5, 2022
idle_clients = np.nonzero(self.client_state)[0]
sampled_clients = np.random.choice(idle_clients,
size=size,
replace=False).tolist()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it possible to make this argument configurable?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you mean replace? It should be set to False to avoid sampling repeated clients.

def __init__(self, client_num):
self.client_state = np.asarray([1] * (client_num + 1))
# Set the state of server (index=0) to 'working'
self.client_state[0] = 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

it is very confusing. Server state is maintained in an array named "client_state"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since 'client_id' is numbered from 1 to M, index=0 for client_state is redundant. Although client_state[0] is used for server_state, it would not be modified during the training process.
Maybe we can declare it in the comments.

Copy link
Collaborator

@joneswong joneswong left a comment

Choose a reason for hiding this comment

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

LGTM

@joneswong joneswong merged commit b1b7af7 into alibaba:master Jul 6, 2022
@xieyxclack xieyxclack deleted the feature/sampler branch July 6, 2022 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants