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

Speech commands explanatory model #1869

Merged

Conversation

swsuggs
Copy link
Contributor

@swsuggs swsuggs commented Jan 25, 2023

Adds explanatory model for poisoning fairness metrics.

Also corrects for an ART bug where triggered audio samples go out of range.

@swsuggs swsuggs changed the title WIP Speech commands explanatory model Speech commands explanatory model Jan 26, 2023
@swsuggs swsuggs requested a review from lcadalzo January 26, 2023 17:41
@swsuggs
Copy link
Contributor Author

swsuggs commented Jan 26, 2023

Most of the non-trivial changes here were needed because the code for the fairness metrics assumed that the explanatory model was a pytorch model, but the speech commands one is tensorflow. It also assumed image data, while speech commands is audio.

This should work for eval6. It makes no pretense to general applicability, meaning It is possible that future model architectures might require additional changes; but that's hard to predict.

@swsuggs
Copy link
Contributor Author

swsuggs commented Jan 30, 2023

@lcadalzo could you review when you get a chance

@swsuggs
Copy link
Contributor Author

swsuggs commented Jan 31, 2023

@lcadalzo the latest commits should address your points, let me know if there's anything else

@lcadalzo
Copy link
Contributor

lcadalzo commented Feb 1, 2023

I had to change "compute_fairness_metric" in the config but now that I'm running the code affected by this PR, I'm seeing the following issue where the script is being killed by the self.explanatory_model() call:

On laptop, using breakpoint():

> /workspace/armory/metrics/poisoning.py(156)get_activations()
-> activation = self.explanatory_model(x_batch, training=False)
(Pdb) n
Killed
I have no name!@1d23884bdaeb:/workspace$

And on remote cluster using a GPU:

2023-02-01 17:57:03 49s INFO     armory.scenarios.poison:compute_explanatory:630 Computing fairness metrics
2023-02-01 17:57:09.852144: F ./tensorflow/core/util/gpu_launch_config.h:160] Check failed: work_element_count > 0 (0 vs. -1591109476)
Aborted (core dumped)

Have you been able to run this code successfully? Are you able to reproduce this?

Note: to make this run much faster, I'm also commenting out the fit() call to avoid waiting on it.

@swsuggs
Copy link
Contributor Author

swsuggs commented Feb 1, 2023

If you are running an audio config you may have to use fit_generator:true to avoid OOM issues

    ...
    "scenario": {
        "kwargs": {
            "fit_generator": true
        },
    ...

Not sure if that's the issue you are hitting; it's not a very descriptive error.

@swsuggs
Copy link
Contributor Author

swsuggs commented Feb 1, 2023

Ok I was finally able to replicate your error by skipping training and not using fit_generator. If I train but don't use fit_generator I get OOM.

@swsuggs
Copy link
Contributor Author

swsuggs commented Feb 1, 2023

@lcadalzo In case I wasn't clear, I don't believe the error you hit is an issue because it can be avoided by using fit_generator.

@lcadalzo
Copy link
Contributor

lcadalzo commented Feb 1, 2023

What's the rationale behind fit_generator being a configurable scenario kwarg?

@swsuggs
Copy link
Contributor Author

swsuggs commented Feb 1, 2023

It runs more slowly but uses less memory. Some users may not find it necessary. See the discussion at 1761 and the info David added to the poisoning doc

Copy link
Contributor

@lcadalzo lcadalzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lcadalzo lcadalzo merged commit eb793c6 into twosixlabs:develop Feb 1, 2023
@swsuggs swsuggs deleted the speech-commands-explanatory-model branch February 2, 2023 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants