-
Notifications
You must be signed in to change notification settings - Fork 297
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
[Autotuner] Plotting utility + test #2394
base: master
Are you sure you want to change the base?
Conversation
luarss
commented
Sep 29, 2024
- Plotting utility
- README
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a call to this script in the test_helper.py
so we test this script.
Signed-off-by: Jack Luar <jluar@precisioninno.com>
Signed-off-by: luarss <jluar@precisioninno.com>
Signed-off-by: Vitor Bandeira <vvbandeira@users.noreply.github.com>
Signed-off-by: luarss <jluar@precisioninno.com>
Signed-off-by: Jack Luar <jluar@precisioninno.com>
Signed-off-by: Jack Luar <jluar@precisioninno.com>
Signed-off-by: Jack Luar <jluar@precisioninno.com>
except Exception as e: | ||
print(f"Error in {fname}: {e}") | ||
continue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we keep a list of the files that were not processed so we can print them at the very end in order for the error not to be buried in the logs? And should the script fail silently in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And should the script fail silently in this case?
Just to clarify, this exception happens when any Autotuner run does not proceed to the "finish" stage, it does not fail silently
Should we keep a list of the files that were not processed so we can print them at the very end in order for the error not to be buried in the logs?
Sure I can add that.
z = np.polyfit(df["timestamp"], df[key], 1) | ||
p = np.poly1d(z) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
z
I assume is the z-axis; what is p
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
z
is the np array of polynomial (ax+b) coefficients, and p
is the actual polynomial function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then let's use more descriptive names, maybe:
z
--> coeff
p
--> poly_func
Signed-off-by: Jack Luar <jluar@precisioninno.com>
Signed-off-by: Jack Luar <jluar@precisioninno.com>
plt.boxplot(df[key]) | ||
plt.ylabel(key) | ||
plt.title(f"{key} Boxplot") | ||
plt.savefig(f"images/{key}-boxplot.png") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's also parametrize the output folder to save images from CI into the reports folder for the design. This way it will be automatically saved to the build artifacts.
- run plot.py for all log files in the autotuner supported design/platforms - parameterised img_dir, so we can store all AT image runs in build artifacts - argparse interface now takes in platform, design, experiment for easier usage - chdir before running plot.py - add docstrings - more error handling Signed-off-by: Jack Luar <jluar@precisioninno.com>
flow/test/test_autotuner.sh
Outdated
--platform ${PLATFORM} \ | ||
--design ${DESIGN_NAME} \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the beginning of this script, you are casting these values to be uppercase, which does not match the path since Linux is case-sensitive. From the CI:
02:32:36 Running Autotuner plotting smoke test
02:32:36 ls: cannot access './flow/logs/SKY130HD/gcd/*/': No such file or directory
02:32:36 No experiments found for plotting
02:32:36 + exit 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we call the plot script and do not find data to plot, this should be an error, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, these are 2 separate errors, will fix them.
Signed-off-by: luarss <jluar@precisioninno.com>
Signed-off-by: Jack Luar <jluar@precisioninno.com>
Signed-off-by: Jack Luar <jluar@precisioninno.com>
Signed-off-by: Jack Luar <jluar@precisioninno.com>