I think there is some error when `update_weights_func`

[nustart0720]

When self.update_weights_func(effective_batch_loss), should we do like this?
`
self.optimizer.zero_grad()

self.update_weights_func(effective_batch_loss)

self.optimizer.step()
`

[Manuscrit]

Well I think that you should have either those in the outside training loop or inside the update_weights_func you provide (or adding them inside the selective_back_propagation function). Current implementation assume that they are called in the outside raining loop:

Outside training loop:

for x, y in data_loader:
    ...
    optimizer.zero_grad()
    with torch.no_grad():
        y_pred = model(x)
    not_reduced_loss = criterion(y_pred, y)
    selective_backprop.selective_back_propagation(not_reduced_loss, x, y)
    optimizer.step()
    ...             

[nustart0720]

@Manuscrit thanks for your reply. I agree with you, but I think in your provided code, if selective_back_propagation doesn't do backprop, optimizer.step() will waste some time doing useless update?

Another question, when I run selective_backprop on resnet18-cifar10, it works well, but It doesn't save time when reach same accuracy compared with traditional 100% selection backpropagation actually. Is there something wrong? I would be very grateful if you could give me any hints.

here is the result when resnet18 training on cifar10, hyperparameters are the same as the author for this paper.it shows acc1 and acc5 when time is going.

[Manuscrit]

@Manuscrit thanks for your reply. I agree with you, but I think in your provided code, if selective_back_propagation doesn't do backprop, optimizer.step() will waste some time doing useless update?

This looks right, I would be interested to know if this cost a lot of time! My implementation isn't very fast, this may be one of the causes.

Is your x axis the number of batch or the time? I think X should be the time instead of the number of batch.