Skip to content

Instrumentation Examples with Probes, Meters, and Writers#1821

Merged
lcadalzo merged 32 commits intotwosixlabs:developfrom
ppark-twosixtech:probe-example
Dec 22, 2022
Merged

Instrumentation Examples with Probes, Meters, and Writers#1821
lcadalzo merged 32 commits intotwosixlabs:developfrom
ppark-twosixtech:probe-example

Conversation

@ppark-twosixtech
Copy link
Copy Markdown
Contributor

@ppark-twosixtech ppark-twosixtech commented Dec 12, 2022

This PR was the outcome of discussions on how to make Probes and Meters more approachable to the user, with the conclusion that more documentation with specific examples would be more beneficial then defining hooking methods in the Probe class for different packages (i.e. pytorch, art, etc.). We determined that defining hooking methods in this manner would not be sustainable since they would have to be maintained by the GARD program should any of those packages change in the future. The templates introduced in this documentation show a flexible approach in how Probes and Meters can fit into other packages/frameworks, and avoids the need to figure out how other packages need to fit into armory. Code and documentation referring to any defined hooking methods for Probes have been removed as well.

…s more details; finished initial draft of Example 2
…use cases; draft is complete with the minimum necessary content; because code examples are from notebook experiments, need to refine content and code that can be executed via armory run with a custom script passed to a config file along with explanations of how to save any Probe\Meter outputs for end-to-end examples
… with user-init block that can be executed with armory run - requires necessary custom files and config not included in this repo
…ossible use cases; draft is complete with the minimum necessary content; because code examples are from notebook experiments, need to refine content and code that can be executed via armory run with a custom script passed to a config file along with explanations of how to save any Probe\Meter outputs for end-to-end examples"

This reverts commit 74365ed.
…use cases; draft is complete with the minimum necessary content; because code examples are from notebook experiments, need to refine content and code that can be executed via armory run with a custom script passed to a config file along with explanations of how to save any Probe\Meter outputs for end-to-end examples
… hooking with user-init block that can be executed with armory run - requires necessary custom files and config not included in this repo"

This reverts commit 28c3152.

Reverting this commit based on discussion with Lucas and the need to focus on Probe examples that work under the assumption that the user is actively changing code to add Probes for a model or attack, as opposed to hooking a Probe to an existing model or attack without modifying the respective code directly - aside from updating the docs (I think at this point I'll rename the existing Probe.md as a different use case and add another doc just for the use case that I dicussed with Lucas), this will not change the fact that the user needs to use the user-init block (I may need to do a pull since the user-init implementation doesn't exist here yet - I've definitely seen it in the repo) with a Meter defined for the Probes that will be added, and it will also not change the need for perhaps a new Meter class that will export probe variables to a pickle file depending on what the user is trying to save (e.g. patch data or image with patch data) rather than a json file. There is also the question of how to deal with size restrictions placed on a Meter (unclear where the restrictions are actually defined - I only know that it exists because of armory logs and the fact that the variable doesn't get saved to the json file when I see size restriction logs).

An additional side note regarding the user-init block for the case that we do decide to add capabilities for Probe hooking without modifying the model or attack code: The way I got it to work in this commit (which will be reverted) is to load the user-init block at the end of all other loads (e.g. model, attack, dataset, metrics etc.) because an instance of a model or attack had to be available to hook to, which I accomplished by passing the Scenario (which contains the model and attack as an attribute) as an argument to a user init function. The existing user-init feature (again not in this branch currently, so will require a git pull and merge), assumes the user-init load happens prior to any other loads, which means none of the instances will be available just yet. Hopefully there will be no issue with this when defining a Meter to accompany any Probe the user may have added to a model or attack code, since the Meter can exist separately from a Probe instance that the Meter will get updates from.
…s an example for probe hooking within a jupyter notebook; will update probe.md to address the user case discussed with Lucas
…nd also add a section on exporting to pickle file
…Probe - these changes were made based on consensus reached with David and Lucas that documentation showing how Probes can be used within existing packages or code is more sustainable for maintenance in the long run rather than having to define functions to fit hooking mechanisms that may or may not exist in other packages into the Probe framework, which will require updates should any of the hooks defined in those packages change
@lcadalzo
Copy link
Copy Markdown
Contributor

@ppark-twosixtech develop is now clean, so once you add a commit to remove probe hook tests, we should expect to see all tests passing

@lcadalzo
Copy link
Copy Markdown
Contributor

this predates your PR, but in metrics.md we say

Strings must be a valid armory metric from armory.utils.metrics

Given our now support for custom metrics, this would be a good time to remove this

@ppark-twosixtech
Copy link
Copy Markdown
Contributor Author

this predates your PR, but in metrics.md we say

Strings must be a valid armory metric from armory.utils.metrics

Given our now support for custom metrics, this would be a good time to remove this

resolved with c24837a

Copy link
Copy Markdown
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.

looks good

@lcadalzo lcadalzo merged commit dbe6ce4 into twosixlabs:develop Dec 22, 2022
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.

Add method hooking to probe functionality Remove hooking functions but document how to use probe with hooks Export patches with export_samples

2 participants