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

[fix]: KeyError Exception thrown by benchmark_size.py when missing baseline data #201

Open
wants to merge 2 commits into
base: embench-2.0-branch
Choose a base branch
from

Conversation

madhu2000u
Copy link

When collecting the relative results, it compares with baseline data. Recently, Depthconv benchmark was added with this commit 07282ee but a baseline data was not added to the baseline-data/size.json.

This commit does not add a baseline data for Depthconv into baseline-data/size.json but rather adds error handling in benchmark_size.py should any new benchmarks get added but a baseline data is not included.

@simonpcook
Copy link
Contributor

If this is triggering when trying to collect data, and the baseline data doesn't get updated, I think this is a very sensible change.

One question though I'd like input from others is if this is merged should this be an error or a warning? I can see arguments either way on this.

@jeremybennett
Copy link
Collaborator

Baseline data must be present for all benchmarks. Its absence should be a catastrophic fail. We need to get the baseline measurement added. I have this on my todo list, but you can propose a value if you have access to the reference platform (STM32F407 @ 16MHz with FPU disabled).

Would you raise an issue to ensure this is dealt with promptly. Thanks.

@madhu2000u
Copy link
Author

Thank you @simonpcook and @jeremybennett for your review and inputs. I do not have the reference platform to propose a value. But I have raised an issue (#202) for this as you suggested.

Also, do you have any suggestions in the code addition to handle such errors or is this good to go for a merge?

Thank you!

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.

3 participants