Conversation
|
Hi Usama, Thanks so much! I've just skimmed over the changes and this looks great! It doesn't look like it will affect any of the other existing code in NNV. I have also just looked over the As far as I can tell, for both If my understanding is accurate, then I think it's safe to merge. Thanks again, Usama! |
|
@sammsaski, you're right; Professor @ttj, please feel free to take a quick look and kindly proceed with the merge if everything looks ok. Thank you all for your time and coordination. |
There was a problem hiding this comment.
Pull request overview
This PR implements weight perturbation functionality for neural network layers to support the MNIST MLP experiment, enabling robustness analysis under weight uncertainty. The changes focus primarily on FullyConnectedLayer.m and Conv2DLayer.m, with supporting utilities in WPutils.m and experimental infrastructure.
Key changes:
- Added
weightPerturbproperty and reachability analysis logic to FullyConnectedLayer and Conv2DLayer for handling weight perturbations - Implemented WPutils static utility class with helper functions for weight perturbation analysis, sampling, and system resource monitoring
- Modified NN.verify_vnnlib to support input reshaping and single average input options
Reviewed changes
Copilot reviewed 14 out of 17 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| code/nnv/engine/nn/layers/FullyConnectedLayer.m | Added weightPerturb property and ImageStar-based reachability logic for weight perturbations |
| code/nnv/engine/nn/layers/Conv2DLayer.m | Added weightPerturb property, GPU memory management, and parallel processing for weight perturbations |
| code/nnv/engine/utils/WPutils.m | New utility class with static methods for weight perturbation, sampling, verification, and system monitoring |
| code/nnv/engine/nn/NN.m | Enhanced verify_vnnlib with input reshaping and added get_layer_indices helper method |
| code/nnv/engine/utils/matlab2nnv.m | Added matlabnet property assignment to NN objects |
| code/nnv/examples/Tutorial/NN/MNIST/weightPerturb/single_layer/conv_expt_any_layer.m | Main experiment script for running weight perturbation experiments |
| code/nnv/examples/Tutorial/NN/MNIST/weightPerturb/single_layer/EXPT.m | Experiment configuration management class with YAML persistence and plotting |
| code/nnv/tests/weightPerturb/test_fc_layer_perturbation.m | Test for FC layer weight perturbation with sampling validation |
| code/nnv/tests/weightPerturb/test_conv_layer_perturbation.m | Test for Conv2D layer weight perturbation with sampling validation |
| code/nnv/examples/Tutorial/NN/Collins_RUL/collins_RUL_tutorial.m | Tutorial demonstrating weight perturbation on Collins RUL CNN |
| code/nnv/examples/Tutorial/NN/MNIST/weightPerturb/single_layer/results/*.yaml | YAML configuration files for experiment specifications |
| .gitignore | Added *.asv to ignore MATLAB autosave files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| % specify a symmetric perturbation to be applied to the whole layer | ||
| % as a fraction of the range of the weights in the layer | ||
| function pert_whole_layer_given_fraction_of_weights_range_call_fromLayer(layer, frac) |
There was a problem hiding this comment.
The function name contains a typo: "pert_whole_layer_given_fraction_of_weights_range_call_fromLayer" should be "pert_whole_layer_given_fraction_of_weights_range_call_from_layer" (missing underscore before "Layer"). This is inconsistent with the naming pattern of other similar functions like "add_pert_call_this_function_from_layer" and "perturb_whole_layer_call_this_function_from_layer".
| function pert_whole_layer_given_fraction_of_weights_range_call_fromLayer(layer, frac) | |
| function pert_whole_layer_given_fraction_of_weights_range_call_from_layer(layer, frac) |
| % @frac: fraction of the weights' range to be applied as | ||
| % perturbation to the whole layer | ||
|
|
||
| WPutils.pert_whole_layer_given_fraction_of_weights_range_call_fromLayer(obj, frac); |
There was a problem hiding this comment.
The function name has an inconsistent naming pattern: "pert_whole_layer_given_fraction_of_weights_range_call_fromLayer" uses a capital "L" in "fromLayer" while other functions in this class use all lowercase with underscores (e.g., "call_this_function_from_layer"). This is also called from FullyConnectedLayer and Conv2DLayer, so fixing it would require updating those files as well.
| WPutils.pert_whole_layer_given_fraction_of_weights_range_call_fromLayer(obj, frac); | |
| WPutils.pert_whole_layer_given_fraction_of_weights_range_call_from_layer(obj, frac); |
| % @frac: fraction of the weights' range to be applied as | ||
| % perturbation to the whole layer | ||
|
|
||
| WPutils.pert_whole_layer_given_fraction_of_weights_range_call_fromLayer(obj, frac); |
There was a problem hiding this comment.
The function name has an inconsistent naming pattern: "pert_whole_layer_given_fraction_of_weights_range_call_fromLayer" uses a capital "L" in "fromLayer" while other functions in this class use all lowercase with underscores (e.g., "call_this_function_from_layer"). This is also called from FullyConnectedLayer, so fixing it would require updating that file as well.
| WPutils.pert_whole_layer_given_fraction_of_weights_range_call_fromLayer(obj, frac); | |
| WPutils.pert_whole_layer_given_fraction_of_weights_range_call_from_layer(obj, frac); |
| if strcmp(nn_name, "MNIST_MLP") | ||
| folder = [nnvroot(), filesep, 'code', filesep, 'nnv', filesep, 'examples', filesep, 'Tutorial', filesep, 'NN', filesep, 'MNIST', filesep, 'weightPerturb', filesep]; | ||
| mnist_model = load([folder 'mnist_model_fc.mat']); | ||
| end |
There was a problem hiding this comment.
Redundant condition: The outer switch statement already filters for nn_name == "MNIST_MLP" at line 29, so the inner if strcmp(nn_name, "MNIST_MLP") check at line 30 is redundant and will always be true. This condition can be removed.
| if strcmp(nn_name, "MNIST_MLP") | |
| folder = [nnvroot(), filesep, 'code', filesep, 'nnv', filesep, 'examples', filesep, 'Tutorial', filesep, 'NN', filesep, 'MNIST', filesep, 'weightPerturb', filesep]; | |
| mnist_model = load([folder 'mnist_model_fc.mat']); | |
| end | |
| folder = [nnvroot(), filesep, 'code', filesep, 'nnv', filesep, 'examples', filesep, 'Tutorial', filesep, 'NN', filesep, 'MNIST', filesep, 'weightPerturb', filesep]; | |
| mnist_model = load([folder 'mnist_model_fc.mat']); |
| end | ||
| end | ||
|
|
||
| if needReshape > 0.1 |
There was a problem hiding this comment.
[nitpick] Unclear conditional check: Using needReshape > 0.1 is an unusual way to check if reshaping is needed. Since needReshape has a default value of 0 and can be 1 or 2, it would be clearer to use needReshape > 0 or needReshape ~= 0 instead of > 0.1. The current threshold of 0.1 seems arbitrary and could be confusing.
| if needReshape > 0.1 | |
| if needReshape > 0 |
Fixed error message Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Thank you for the Copilot review, Professor @ttj. I integrated some of the refinements; there were no logical issues (there was a string + number concatenation issue in an |
|
Thanks Usama! I am going to merge this now, please don't make further edits |
There was a problem hiding this comment.
after merging, I tried to run this test, and got the following error. is this load_images function somewhere?
please make a new pull request with it if some files were missed
Running test_conv_layer_perturbation
================================================================================
Error occurred in test_conv_layer_perturbation/test_conv_layer_perturbation and it did not run to completion.
---------
Error ID:
---------
'MATLAB:UndefinedFunction'
--------------
Error Details:
--------------
Undefined function 'load_images' for input arguments of type
'SeriesNetwork'.
Error in test_conv_layer_perturbation (line 33)
[images, labels] = load_images(database = "mnist", ...
================================================================================
.
Done test_conv_layer_perturbation
__________
Failure Summary:
Name Failed Incomplete Reason(s)
==========================================================================================
test_conv_layer_perturbation/test_conv_layer_perturbation X X Errored.
ans =
TestResult with properties:
Name: 'test_conv_layer_perturbation/test_conv_layer_perturbation'
Passed: 0
Failed: 1
Incomplete: 1
Duration: 1.9026
Details: [1×1 struct]
Totals:
0 Passed, 1 Failed, 1 Incomplete.
1.9026 seconds testing time.
|
My apologies, it appears that my MATLAB was using this function from the path of another copy of NNV on the machine. I will create another PR with this function in the |
|
Professor @ttj, could you please let me know whether I should go ahead and add a description of the MNIST MLP experiment, along with corresponding plots? Once I add it, @sammsaski can revise or adjust it as needed. I wasn’t entirely sure whether you preferred that I draft the initial description or that @sammsaski do so. |
|
I think I can speak for @ttj here. Please feel free to add an initial description of the experiment along with the corresponding plots! |
|
Sure, I'll do so tomorrow then, while limiting the experiment to the last 3 layers of the MLP and using 100 images as in the code provided. |
|
yes, thanks Usama, also please make sure to add yourselves as co-authors given the contribution, thanks for the updates too! I think everything ran fine, so I think will be great to include, thanks for all the hard work |
|
Sure, Professor. Just one more thing I thought I'd clarify: while my instance of MATLAB was using |
3 participants
Hello Professor @ttj and @sammsaski,
Fortunately, I was able to finish making changes for the FC layer quickly. I maintained the entirety of changes in
FullyConnectedLayer.mincluding the multi-layer case, but these are primarily two changes: an object variableweightPerturband the weight perturbation addition to the reachability analysis code which runs only whenweightPerturbis non-empty. I added the required helper functions in a utility fileutils/WPutils.mas static functions, and modified the experimentconv_expt_any_layer.maccordingly. The instructions for launching the experiment remain the same as described in the original PR.Unless I am fogetting something, the only additional change required to match up to the description in the paper would be the update to
Conv2DLayer.m, which I can go ahead and try if it is desirable simultaneously, but that is not needed for the MNIST MLP experiment. That would be a change similar toFullyConnectedLayer.m, i.e., it uses aweightPerturbobject variable and reachability code that runs only when that variable is non-empty. Please feel free to take a look at the changes inConv2DLayer.min the original pull request and let me know if it is desirable at the moment.