Bug Fix [Issue #355]

[SHAO-Jiaqi757]

Description

How has this been tested?

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality) [BUG] consistent_clients config not works in sampler: mixed_label_quantity_noniid #355
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
  Issue#355

• To make consistent_clients usable, set self.consistent_clients = Config().data.consistent_clients.
  If the configuration has consistent_clients, then consistent_client_size is useless since it is only used for randomly sampling a set of consistent_clients.

Changes:

    if hasattr(Config().data, "consistent_clients"):
        self.consistent_clients = Config().data.consistent_clients
    else:
        consistent_clients_size = Config().data.consistent_clients_size
        self.consistent_clients = np.random.choice(
        list(range(total_clients)), size=consistent_clients_size, replace=False
    )

• To make client_id consistent (the original code used client_id for the client index), there are some modifications for client_id to client_idx.
  The consistent_clients in configuration can be set with a list of client ids.

• Testing

client = Client(trainer=trainer)
sampler = client.sampler.get()
indices = sampler.indices
labels = [client.trainset[i][1] for i in indices]
counter = Counter(labels)
print(f"client id # {client.client_id} has trainsets stats {counter}")

Output:

• Note that the consistent_clients = [1, 2] and anchor_classes = [3, 4, 9]
  For the index based clients_contain_classes, key = client_id -1

clients_contain_classes:  {0: [3, 4, 9], 1: [3, 4, 9], 2: [2, 5, 3], 3: [3, 6, 8], 4: [4, 0, 5], 5: [5, 7, 0], 6: [6, 7, 1], 7: [7, 5, 1], 8: [0, 7, 1], 9: [1, 8, 0]}

client id # 1 has trainsets stats Counter({9: 2975, 4: 1948, 3: 1533})
client id # 2 has trainsets stats Counter({9: 2974, 4: 1947, 3: 1533})
client id # 3 has trainsets stats Counter({2: 5958, 3: 1533, 5: 1356})
client id # 4 has trainsets stats Counter({6: 2959, 8: 2926, 3: 1532})
client id # 5 has trainsets stats Counter({4: 1947, 0: 1481, 5: 1355})
client id # 6 has trainsets stats Counter({7: 1567, 0: 1481, 5: 1355})
client id # 7 has trainsets stats Counter({6: 2959, 1: 1686, 7: 1566})
client id # 8 has trainsets stats Counter({1: 1686, 7: 1566, 5: 1355})
client id # 9 has trainsets stats Counter({1: 1685, 7: 1566, 0: 1481})
client id # 10 has trainsets stats Counter({8: 2925, 1: 1685, 0: 1480})

Checklist:

• My code has been formatted using Black and checked using PyLint.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.

The documentation may not need to change according to comments, where the client is labeled by client_id rather than client_idx.

[netlify]

✅ Deploy Preview for platodocs canceled.

Name	Link
🔨 Latest commit	0a6af60
🔍 Latest deploy log	https://app.netlify.com/sites/platodocs/deploys/65373e63cf72250008f0c287

[CSJDeveloper]

First, the reported Issue#355 is not a bug because the mixed_label_quantity_noniid.py is designed to only accept consistent_clients_size and choose the "consistent_clients" randomly.

Thus, what the author did in the PR was to add a new feature to mixed_label_quantity_noniid.py by also including the consistent_clients in the configuration.

Therefore, you may need to change the label to be the enhancement.

Second, I, personally, do not think such an 'enhancement' is a good idea because when there are 1000 clients, why does a user want to set the "consistent_clients" (maybe 100 clients) one by one in the configuration file?

[SHAO-Jiaqi757]

According to file description, consistent_clients can be configured.
However, there seems to be a lack of alignment between the file's description and its actual usage. This discrepancy can lead to confusion for users and developers alike.

Furthermore, the primary objective of this configuration is to simulate research scenarios, especially with a limited number of clients. In such cases, having precise control over specific clients becomes crucial. Instead, randomly selecting 30 out of 100 clients to be consistent_clients may not effectively capture the desired impact, which might not be suitable in a research setting where the focus is on understanding the nuances of specific client behaviors.

It would be beneficial to provide a more deterministic way of selecting consistent_clients or allowing users to specify which clients they want to be consistent. This would offer more flexibility and precision in research simulations.

[CSJDeveloper]

According to file description, consistent_clients can be configured. However, there seems to be a lack of alignment between the file's description and its actual usage. This discrepancy can lead to confusion for users and developers alike.

Furthermore, the primary objective of this configuration is to simulate research scenarios, especially with a limited number of clients. In such cases, having precise control over specific clients becomes crucial. Instead, randomly selecting 30 out of 100 clients to be consistent_clients may not effectively capture the desired impact, which might not be suitable in a research setting where the focus is on understanding the nuances of specific client behaviors.

It would be beneficial to provide a more deterministic way of selecting consistent_clients or allowing users to specify which clients they want to be consistent. This would offer more flexibility and precision in research simulations.

I get your point, and we reached an agreement that the current adjustment is more like an enhancement. Right?

If so, it would be great to accept your revision on consistent_clients.

I noticed that you also adjusted the 'client_id' or 'client_idx' to make it clear. My suggestion is that could you please change it back as if we want to change some code under samplers/, all samplers should be revised or tested accordingly.

Therefore, in this PR, it would be great to only maintain the enhancement. But for the change made on client_id, we can move it to another PR if you want.

[SHAO-Jiaqi757]

According to file description, consistent_clients can be configured. However, there seems to be a lack of alignment between the file's description and its actual usage. This discrepancy can lead to confusion for users and developers alike.
Furthermore, the primary objective of this configuration is to simulate research scenarios, especially with a limited number of clients. In such cases, having precise control over specific clients becomes crucial. Instead, randomly selecting 30 out of 100 clients to be consistent_clients may not effectively capture the desired impact, which might not be suitable in a research setting where the focus is on understanding the nuances of specific client behaviors.
It would be beneficial to provide a more deterministic way of selecting consistent_clients or allowing users to specify which clients they want to be consistent. This would offer more flexibility and precision in research simulations.

I get your point, and we reached an agreement that the current adjustment is more like an enhancement. Right?

If so, it would be great to accept your revision on consistent_clients.

I noticed that you also adjusted the 'client_id' or 'client_idx' to make it clear. My suggestion is that could you please change it back as if we want to change some code under samplers/, all samplers should be revised or tested accordingly.

Therefore, in this PR, it would be great to only maintain the enhancement. But for the change made on client_id, we can move it to another PR if you want.

I agree that this change can be viewed as "feature enhancement" and I will check other client_idx in sampler for further clarity.
Could you please help me to know what should we do next for this PR?

[CSJDeveloper]

According to file description, consistent_clients can be configured. However, there seems to be a lack of alignment between the file's description and its actual usage. This discrepancy can lead to confusion for users and developers alike.
Furthermore, the primary objective of this configuration is to simulate research scenarios, especially with a limited number of clients. In such cases, having precise control over specific clients becomes crucial. Instead, randomly selecting 30 out of 100 clients to be consistent_clients may not effectively capture the desired impact, which might not be suitable in a research setting where the focus is on understanding the nuances of specific client behaviors.
It would be beneficial to provide a more deterministic way of selecting consistent_clients or allowing users to specify which clients they want to be consistent. This would offer more flexibility and precision in research simulations.

I get your point, and we reached an agreement that the current adjustment is more like an enhancement. Right?
If so, it would be great to accept your revision on consistent_clients.
I noticed that you also adjusted the 'client_id' or 'client_idx' to make it clear. My suggestion is that could you please change it back as if we want to change some code under samplers/, all samplers should be revised or tested accordingly.
Therefore, in this PR, it would be great to only maintain the enhancement. But for the change made on client_id, we can move it to another PR if you want.

I agree that this change can be viewed as "feature enhancement" and I will check other client_idx in sampler for further clarity. Could you please help me to know what should we do next for this PR?

Thank you very much for your work!

For this PR, could you please just remove your changes in sampler_utils.py? By doing so, this PR can contain the enhancement.

If you want to have more work on the 'client_id' of the sampler, you can create a PR specific for this. Thus, we can have more discussion there when necessary.