Skip to content
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

AttackerModel should not perform normalization #38

Open
NoahAmsel opened this issue Apr 18, 2020 · 1 comment
Open

AttackerModel should not perform normalization #38

NoahAmsel opened this issue Apr 18, 2020 · 1 comment
Assignees
Labels
enhancement New feature or request

Comments

@NoahAmsel
Copy link

I think it would be cleaner if the normalization step were baked into the underlying model rather than being performed by the AttackerModel. Conceptually, the act of normalizing inputs is not related to the AttackerModel. Combining them causes problems. For instance, I'm trying to wrap AttackerModel around a network that already does normalization. My proposed change is to make the dataset optional in the initialization of an AttackerModel:

    def __init__(self, model, dataset=None):
        super(AttackerModel, self).__init__()
        if dataset is None:
            # we can skip the normalize step if we want
            self.normalizer = ch.nn.Identity()
        else:
            self.normalizer = helpers.InputNormalize(dataset.mean, dataset.std)
        self.model = model
        self.attacker = Attacker(model, dataset)
@andrewilyas
Copy link
Collaborator

This is a good idea! Am a little swamped right now, but will get it into the next release. Pull requests into the develop branch also welcome :)

@andrewilyas andrewilyas added the enhancement New feature or request label Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants