-
Notifications
You must be signed in to change notification settings - Fork 8
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
Normalize data #36
Normalize data #36
Conversation
This wasn't enough. Training is perhaps a bit faster in the beginning but no big difference at convergence. Besides the clipping differences, there is a possible issue with the normalization of the warping field (on the order of image width in number of pixels). I'll:
I'll make another PR if that turns out to be the issue. |
Might want to set normalization to False by default as to not complicate matters. Running some experiments to see that performance isn't hurt too bad without normalization (but with fixes from #37 ). |
Regarding the transform order: at a very high level, I think a The more practical question of what to do for backdoors is a bit less clear. I agree that applying backdoors first and then augmentations/normalization is probably more realistic from a backdoor threat model. But I'd guess that applying the backdoor last makes things easier in practice? If so, I think that'd be fine too. Though if e.g. learning a WaNet backdoor turns out to be much harder for the model if we just do some augmentations after inserting the trigger, that's also kind of interesting from an image backdoor defense perspective---easiest defense of all time, already built into any realistic training process :P I think it's much less important for our purposes but might still be worth pointing out somewhere if true |
Also added a debug message as warning. Could consider to have this be an actual warning, but as this is an active choice I figured it wouldn't need to be more verbose than a debug message.
30bbd3c
to
00f3d53
Compare
This should now mostly have no actual changes for default value right now. Can either be:
|
I agree that it could be interesting to investigate, but I don't consider it a priority. My guess is that for the most part adding augmentations after poisoning and then train would probably lead to both harder to learn and harder to detect backdoors (similar to adding noisy warping for WaNet). |
Looks good at a glance, planning to go over it in more detail tomorrow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, basically looks good (though I haven't tried training anything with normalization). Only thing I noticed are a few type annotations that look wrong. Fine by me to remove type annotations or use Any
if they're too annoying to fix, just want to make sure they're not misleading. Feel free to merge once you've gone over the corresponding inline comments!
src/cupbearer/data/transforms.py
Outdated
|
||
import torch | ||
import torchvision.transforms.functional as F | ||
|
||
Sample = tuple[torch.Tensor] | tuple[torch.Tensor, Any] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In practice, this is correct, but it seems inconsistent with the type annotations for __rest_call__
, should be tuple[torch.Tensor, Any, ...]
for the second option. At that point, we might as well use tuple[torch.Tensor, ...]
. Though I think arguably we should have a more consistent convention here anyway, not sure what it should be
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had tuple[torch.Tensor, *tuple[Any, ...]]
before I realized that that gives SyntaxError in Python<3.11. Would be an option if we bump version requirements.
Alternatively, perhaps be restrictive to tuple[torch.Tensor, ...]
instead would be best.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made the typehint less specific than it should be for call but type checkers should be correct as long as they pick up on sample being split and sent to the separate methods form img and rest.
This is probably the limitation that remains for good WaNet noise performance.
TODO:
Some backdoors like corner it makes a big difference if they are normalized or not. I can see two different arguments for ordering: