-
Notifications
You must be signed in to change notification settings - Fork 67
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
Generic Masked Patch Attack #1904
Conversation
|
||
|
||
@dataclass | ||
class PatchMask: |
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'm not sure if this file is the correct place for this class since we do not have a dedicated 'attack utils' file. Should this be moved to its own file?
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.
Should be moved to be within armory.utils
@christopherwoodall Still WIP, not worth you trying to debug the current issue with patch_base_image but feel free to address other comments; will request review by EOW with fixes |
…utils. Split armory cli tools into separate files.
…t for patch shape kwargs; working attack
@mwartell I am running into python package issues when installed not in editable mode but it works as expected when it is. Specifically armory cannot find |
The culprit is an overzealous exclusion in pyproject.toml I'm looking for the syntax to fix that, likely Hatch claims to use the gitignore pattern rules which is a good choice.
Therefore
will exclude |
armory/cli/tools/generate_shapes.py
Outdated
def generate_shapes(command_args, prog, description): | ||
from armory.utils.shape_gen import ( # move to lazy load to avoid PIL import error | ||
Shape, | ||
) |
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.
Is there a better way to handle this? In particular, Pillow is listed under engine
and develop
but when you install armory-testbed standard it is not included and will error when trying to run armory --help
. This fixes that problem but creates another that if generate_shapes
is called at runtime from an environment without PIL the error message will not be useful. However since this is strictly code that is called from either the cli in which the user will know how to fix the issue or from the carla_adversarial_patch_pytorch
code which must be running with engine
installed and PIL must also be installed I left it this way.
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.
We had wanted to make the armory launcher clean so that the host could have a thin (no ML framework install) loader that relied on the contents of the container for the heavy libraries). This was a big motivation for keeping the dependencies
lean. Since Armory v23.x has dropped containers and the two-stage armory launch, and with Armory v0.x.x having an informal end-of-life in under 18 months, I think we just don't care that much any more.
Do you need PIL in pyproject dependencies
? Then put it there, it isn't a big deal anymore. @christopherwoodall care to render an opinion?
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.
Totally in agreement that all required libraries should be in the dependencies section.
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.
Let's get those CI tests passing, otherwise, it looks good to me.
armory/cli/tools/generate_shapes.py
Outdated
def generate_shapes(command_args, prog, description): | ||
from armory.utils.shape_gen import ( # move to lazy load to avoid PIL import error | ||
Shape, | ||
) |
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.
Totally in agreement that all required libraries should be in the dependencies section.
The goal of this PR is to implement a generic masked patch module to be integrated into existing attacks with minimal code change to the attack module
How to use this code
See
To illustrate what this does see the example below:
Example Run
Attack kwargs from config
Resultant image