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

Feature/validation steps #100

Open
wants to merge 14 commits into
base: dev
Choose a base branch
from
Open

Feature/validation steps #100

wants to merge 14 commits into from

Conversation

jmonks-amd
Copy link
Collaborator

@jmonks-amd jmonks-amd commented Jul 17, 2024

The PR enables optional verification for all finn-examples models. The verification is performed by running the model against a known input, then comparing the produced output against a known output to check if the values match, returning a success or fail depending on the result. The verification steps are run at various points throughout the build, after its corresponding build step has completed.

The verification can be enabled by setting the following environment variables:

  • VERIFICATION_EN : set to 1 to enable verification. If unset or set to any other value, verification will be disabled
  • FINN_EXAMPLES_ROOT : set as the path to the root of your finn-examples repository
  • VERIFICATION_IO : set as the path to the folder containing the input and expected output .npy files for verification

Once these variables are set, the FINN build can be run and verification will be used within the build.

This PR depends on Xilinx/finn#1140

Copy link
Collaborator Author

@jmonks-amd jmonks-amd left a comment

Choose a reason for hiding this comment

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

Some initial comments. Will have another look later/tomorrow as needed

build/cybersecurity-mlp/build.py Show resolved Hide resolved
build/cybersecurity-mlp/build.py Show resolved Hide resolved
build/gtsrb/build.py Outdated Show resolved Hide resolved
build/gtsrb/build.py Outdated Show resolved Hide resolved
build/gtsrb/build.py Outdated Show resolved Hide resolved
build/mobilenet-v1/build.py Show resolved Hide resolved
build/resnet50/build.py Outdated Show resolved Hide resolved
build/resnet50/build.py Outdated Show resolved Hide resolved
build/vgg10-radioml/build.py Show resolved Hide resolved
build/vgg10-radioml/build.py Outdated Show resolved Hide resolved
@jmonks-amd
Copy link
Collaborator Author

Looks good to me!

platforms_to_build = zynq_platforms + alveo_platforms


def custom_step_update_model(model, cfg):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we can move this function to a central point, since it is used in multiple places. Either to finn or to qonnx, we can sync offline about this.

Due to manual edits I made, some outdated changes were favoured in the merge.

This reverts commit bc6c296, reversing
changes made to 44d1f35.
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.

4 participants