# Contributing to the Flower Github repository

**URL:** <https://discuss.flower.ai/t/contributing-to-the-flower-github-repository/397>\
**Category:** Flower Help - Beginners\
**Tags:** baseline\
**Created:** [November 2, 2024, 9:09am UTC](https://discuss.flower.ai/t/contributing-to-the-flower-github-repository/397 "2024-11-02T09:09:13Z")\
**Posts on this page:** 1\
**Showing post:** 4

<div class="post-metadata">

**Author:** ![mistersunshine](https://avatars.discourse-cdn.com/v4/letter/m/e9bcb4/32.png) [@mistersunshine](https://discuss.flower.ai/u/mistersunshine)\
**Post date:** [November 8, 2024, 9:09pm UTC](https://discuss.flower.ai/t/contributing-to-the-flower-github-repository/397/4 "2024-11-08T21:09:14Z")

</div>

Hi @javier and @wittenator,

Thank you both for your answers. It’s finally week-end, so i’ve time to respond.

First of all, i congrats you @wittenator for your implementation, it’s not easy to implement this algorithm in the Flower simulation environnement and i find it great on what you did for the `marshal_numpy` and `unmarshal_numpy` functions.  
I still have few questions on the equations you wrote, since i also implemented those equations but in a different way.

- For c\_global equations (`strategy.py`)

Your computation of c\_global is as follow :

```auto
aggregated_control = [(float(num_clients)/self.total_num_clients) * reduce(np.add, c_delta) / num_clients for c_delta in zip(*c_delta_results)]
self.global_control = aggregated_control

```

And it’s defined in the original article as follow :  
 ![image_2024-11-08_212632783](https://europe1.discourse-cdn.com/flex013/uploads/flower/original/1X/b1ccb1225df454c31a3ac0b034d9b27a6bc026aa.png)  
I think you miss the original weight on the aggregated\_control. And you erase them each round, i did it like that on my side :

```auto
for i in range(len(self.c_global)):
  c_delta_avg = c_delta_sum[i] / self.num_clients
  self.c_global[i] += torch.tensor(c_delta_avg)

```

Have you some sources on why you did it like that ? Maybe i missed some mathematical proof on what you did.

- For the weight regularization (`model.py`)

I’ve seen some implementation also doing what you did, so i’m not a hundred percent sure.

Your computation of the weight regularization is as follow :

```auto
for _ in range(epochs):
    for batch in trainloader:
        images = batch["img"]
        labels = batch["label"]`Preformatted text`
        if images.shape[0] == 1:
            # Skip batches with a single image
            continue
        optimizer.zero_grad()
        loss = criterion(net(images.to(device)), labels.to(device))
        loss.backward()
        for (name,param), c, c_i in zip(
            net.state_dict().items(), global_control, local_control
        ):
            if param.requires_grad:
                # The global control does not have batchnorm dimensions at the beginning, but is zero at this point in time
                if c.shape == c_i.shape:
                    param.grad.data += torch.tensor(c - c_i).to(device)
                else:
                    param.grad.data += torch.tensor(-c_i).to(device)
        optimizer.step()

```

And it’s defined in the original article as follow :

 ![image_2024-11-08_214021038](https://europe1.discourse-cdn.com/flex013/uploads/flower/original/1X/de9ca90246760225f7ce55360aa0ed220ce97292.png)

I feel like it is expressed as weight regularization should be outside of batch loop. And i find the learning and and y\_i missing from your loop.

I implemented it like that :

```auto
# Train the global model with local data
for _ in range(epochs):
    prebatch_params = [param.detach().clone() for param in self.net.parameters()]
    
    for batch in trainloader:
        images = batch["image"]
        labels = batch["label"]
        self.optimizer.zero_grad()
        self.criterion(net(images.to(device)), labels.to(device)).backward()
        self.optimizer.step()

    # Adds Scaffold computation of c_diff in parameters
    for param, y_i, c_l, c_g in zip(self.net.parameters(), prebatch_params, c_local, c_global):
        param.grad.data = y_i - (learning_rate * (param.grad.data - c_l + c_g))

```

I did it this way so my weights would be updated after i learned from my local gradient g\_i(y\_i). I’m still unsure about my implementation ! Since i saw the few implementation available on Internet did it your way.

That’s all for my remarks.

Just to be aware, be careful if you use the Flower simulation environnement, because you store c\_local in the client constructor but it is erased each rounds. On my side i had to serialize every c\_local in a file.

See [this post](https://discuss.flower.ai/t/how-to-avoid-flower-next-from-destroying-my-model-on-every-fit-and-every-evaluate/356) for more information.

For my full implementation you can find it [here](https://github.com/ThomasCalonge/scaffold-flower) on Github. I will soon push force a whole new version, so be careful if you see any errors in my code, it may already been corrected.

Would be a pleasure to read more from you.

Best regards,  
Mr. Sunshine.

---

_[View the full topic](https://discuss.flower.ai/t/contributing-to-the-flower-github-repository/397)._