Wrong Implementation

[nutszebra]

Hi, I use your GAP for my model analysis and had read your code. I found that your code modifies the value in the matrix by directly accessing data property (e.g., here and here) As far as I understand, this implementation causes to generate unexact gradients and I assume this is not what you expect.

My question is that "is this your implementation correct and the results on paper are based on this code?".

Thanks.

[isaykatsman]

Hi @nutszebra, thanks for your interest in our work.

If you were referring specifically to the line of code with clipping: Yes, you are correct in that we clip weights and this causes gradients to be inexact. You are incorrect in that this is not what we expected. Weight clipping is a fairly common way to enforce some Lp norm constraint (https://arxiv.org/pdf/1701.07875.pdf, https://arxiv.org/pdf/1610.08401.pdf), and it does come at the cost of producing approximate gradients. However, even with approximate gradients, the optimization is able to converge to reasonably good universal perturbations within a reasonable number of iterations. All perturbations in the paper are generated with this method, are within the specified Lp norm, and achieve the specified fooling ratios.

If you would prefer not to do gradient clipping in general, you can instead add a norm loss to your optimization in order to encourage the perturbation to have low-norm (this was the approach taken by the C&W attack, https://arxiv.org/pdf/1608.04644.pdf); however, this comes at the cost of no longer being able to provide Lp norm guarantees at test time (for iterative per-image methods). For this paper, we wanted to stay within the Lp norm for comparison with previous work (https://arxiv.org/pdf/1610.08401.pdf) so we chose to clip.

If you were referring to .data usage: We use .data instead of just the tensor itself primarily because the original implementation was with PyTorch 0.3. In this case, the usage is correct. The reason .data was deprecated in more recent versions is because it can introduce gradient bugs, as you mentioned. For example, in-place modifications post-loss variable definition can introduce bugs. To be clear, I have given an example of this below:

Correct (no in-place operations):

>>> a = torch.tensor([2.0], requires_grad=True)
>>> b = torch.tensor([3.0], requires_grad=True)
>>> loss = a * b
>>> b = b + 1
>>> loss.backward()
>>> a.grad
tensor([3.])

Bug (bad in-place operations):

>>> a = torch.tensor([2.0], requires_grad=True)                                 
>>> b = torch.tensor([3.0], requires_grad=True)                                 
>>> loss = a * b
>>> b.data[0] = b.data[0] + 1
>>> loss.backward()
>>> a.grad
tensor([4.])

Basically, if you in-place change variables by using .data that your loss depends on after defining your loss, you can run into some bad implementation issues. In order to avoid bugs like the above, you are in general correct about avoiding this usage, particularly as it is convention in PyTorch V1+.

[nutszebra]

Thank you for your reply.

I was referring to data usage case and above examples are the very understandable and clear explanation. I didn't know that this type of usage is no problem in some cases (I'm very surprised). Again, I really appreciate your quick reply and proper explanation!

[nutszebra]

I'm very sorry to ask the question again.

a = torch.tensor([2.0], requires_grad=True)
b = torch.tensor([3.0], requires_grad=True)
c = b ** 2
c *= 4
loss = a * c
loss.backward()
print(b.grad)

b.grad.zero_()
print(b.grad)

a = torch.tensor([2.0], requires_grad=True)                                 
b = torch.tensor([3.0], requires_grad=True)                                 
c = b ** 2
c.data[0] *= 4
loss = a * c
loss.backward()
print(b.grad)

Above code causes different grad even though loss definition is at the end. I'm still very confused with your explanation and this result.

[isaykatsman]

@nutszebra thank you for your reply. Initially, I just gave the post-loss modification as an example of where it could go wrong. There are definitely other cases where using .data in the current version of PyTorch can go wrong, particularly if we use it with non-leaf variables, as you did in your example. Because we are directly multiplying c.data[0] by 4, separate Mul and MulBackward nodes will not be created by autograd and the resulting gradient will be a factor of 4 smaller than it should be (12 instead of 48). My mistake in looking at this code when replying to you was assuming delta_im was a leaf variable (as it usually is in more typical attacks like I-FGSM), as modifying leaf variables directly with .data is usually fine (pre-loss computation) since we only use the values of leaf variables in gradient computations, e.g.:

Correct (no in-place computation on leaf node):

>>> a = torch.tensor([2.0], requires_grad=True)
>>> b = torch.tensor([3.0], requires_grad=True)
>>> b = b * 4
>>> loss = a * b
>>> loss.backward()
>>> a.grad
tensor([12.])

Also correct (in-place computation on leaf node):

>>> a = torch.tensor([2.0], requires_grad=True)
>>> b = torch.tensor([3.0], requires_grad=True)
>>> b.data[0] = b.data[0] * 4
>>> loss = a * b
>>> loss.backward()
>>> a.grad
tensor([12.])

Upon further inspection of this code, delta_im is clearly not a leaf variable (I have not looked at this code in a while, this is my bad). Hence with the current version of PyTorch this is clearly a bug given that the two lines you pointed out will result in incorrect gradient propagation. I tried running the example attack from the README and was unable to reproduce the results I was getting before. Upon downgrading to PyTorch 0.3, I was able to reproduce the results. Formerly, gradient information that was being tracked with .data is no longer being tracked now (probably due to the fact that the Variable/Tensor abstraction has been completely thrown out), and this makes this implementation of normalize_and_scale buggy with the current version of PyTorch. Funny enough, upon checking my local implementation of normalize_and_scale in another, more recent, project I found:

def normalize_and_scale(delta_im, mode='train'):
    if opt.foolmodel == 'incv3':
        delta_im = nn.ConstantPad2d((0,-1,-1,0),0)(delta_im) # crop slightly to match inception

    delta_im = delta_im + 1 # now 0..2
    delta_im = delta_im * 0.5 # now 0..1

    # normalize image color channels
    for c in range(3):
        delta_im[:,c,:,:] = (delta_im[:,c,:,:].clone() - mean_arr[c]) / stddev_arr[c]

    # threshold each channel of each image in deltaIm according to inf norm
    # do on a per image basis as the inf norm of each image could be different
    bs = opt.batchSize if (mode == 'train') else opt.testBatchSize
    for i in range(bs):
        # do per channel l_inf normalization
        for ci in range(3):
            l_inf_channel = delta_im[i,ci,:,:].detach().abs().max()
            mag_in_scaled_c = mag_in/(255.0*stddev_arr[ci])
            gpu_id = gpulist[1] if n_gpu > 1 else gpulist[0]
            delta_im[i,ci,:,:] = delta_im[i,ci,:,:].clone() * np.minimum(1.0, mag_in_scaled_c / l_inf_channel.cpu().numpy())

    return delta_im

Clearly, this was my intended implementation for this repo as well, but due to negligence I forgot to update this repo. I just pushed the changes and confirmed that I am able to reproduce the example result in the README once again with PyTorch V1.1.0 and this code.

Thank you for bringing this to my attention; I genuinely appreciate it.

[nutszebra]

Hi, @isaykatsman.

I'm surprised that directly modifying data creates the graph in Pytorch 0.3 (thank you for letting me know). Plus, I felt relieved to hear that the results on the paper are correct.

Thank you for answering my question and updating the repo!