-
Notifications
You must be signed in to change notification settings - Fork 0
Make Projector batch aware #120
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
Conversation
|
Do you have a plan for modality-aware? |
|
Nope. |
| """Apply a list of perturbation modifier.""" | ||
|
|
||
| def __init__(self, projectors: List[Projector]): | ||
| def __init__(self, projectors: list[Projector]): |
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.
Do you think it's better to make it a dictionary so that it's more easily configurable in Hydra? Like that in Enforcer:
Line 100 in 70e62d9
| def __init__(self, constraints: dict[str, Constraint] | None = None) -> None: |
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 think that should live in another PR.
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.
My read of the Projectors class is that they need a wholesale redesign because there's a lot of mixing of functionality instead of focusing on composition.
|
I have made a shared modality-batch-aware mechanism in #115 for Initializer, Projector and Composer. |
Okay. Is the idea that this PR should be updated to use that? I also saw there was an opportunity to DRY the logic but wanted to wait until after we get everything merged. I'll try to read through what you wrote though. |
|
@mzweilin: I think it's better to merge this now than wait for the generic dispatch to be done since it updates the tests to conform to the existing interface. |
mzweilin
left a comment
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.
LGTM
What does this PR do?
The current
Projectoris not batch/tuple aware, which is necessary for attacks on datasets where input images have different shapes (e.g., COCO). This PR makes it so.Type of change
Please check all relevant options.
Testing
Please describe the tests that you ran to verify your changes. Consider listing any relevant details of your test configuration.
make testBefore submitting
pre-commit run -acommand without errorsDid you have fun?
Make sure you had fun coding 🙃