Change default plot sizes for radical-analytics-inspect plots#205
Change default plot sizes for radical-analytics-inspect plots#205andre-merzky merged 9 commits intodevelfrom
radical-analytics-inspect plots#205Conversation
There was a problem hiding this comment.
Summary of Changes
Hello @andre-merzky, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request primarily focuses on adjusting the default output size and appearance of radical-analytics-inspect plots. The core intent is to discourage the use of these plots as publication-quality figures by making them smaller and more suited for quick overviews, while still ensuring they are presentable. It also introduces an environment variable for users to customize the plot resolution and includes general code hygiene improvements.
Highlights
- Default Plot Size Reduction: The default width for
radical-analytics-inspectplots has been significantly reduced from 500 to 252 pixels. This change aims to make the plots less suitable for direct publication and more appropriate for quick overviews, aligning with their intended purpose. - Configurable Plot Resolution: Introduced support for configuring plot resolution via the
RADICAL_ANALYTICS_RESOLUTIONenvironment variable. This allows users to override the new default size if a different resolution is desired. - Python 3 Shebang Updates: All
rp_inspectscripts have had their shebangs updated from#!/usr/bin/env pythonto#!/usr/bin/env python3, ensuring explicit compatibility with Python 3 environments. - Plot Label and Legend Refinements: The
plot_conc.py,plot_dur.py, andplot_state.pyscripts now use a refactoredevent_liststructure that includes explicit, simplified labels for plot series. Legend positioning and column counts have also been adjusted across several plot types for better readability with the new default sizes. - Utilization Plot Enhancements: The
plot_util_2.pyscript received updates including refined resource tracking (now explicitly supporting both CPU and GPU), simplified utilization labels, and improved data handling to skip plotting if insufficient data is available. The Y-axis limit for percentage plots was also corrected to 100%.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request primarily focuses on adjusting the default plot sizes for radical-analytics-inspect plots and making the resolution configurable via an environment variable. It also includes updates to use python3 and some minor cleanup and adjustments in the plotting scripts, such as simplifying event list structures and correcting a y-axis limit. The changes generally improve maintainability by introducing a configurable resolution and cleaning up some data structures. A few minor cleanup suggestions are provided.
bin/rp_inspect/plot_state.py
Outdated
| ax.plot(np_data[:,0], np_data[:,(1 + e_idx)], | ||
| label=to_latex(ru.event_to_label(event_list[e_idx]))) | ||
| for idx,item in enumerate(event_list): | ||
| print(idx, item[1], item[0]) |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## devel #205 +/- ##
=======================================
Coverage 23.11% 23.11%
=======================================
Files 7 7
Lines 1194 1194
=======================================
Hits 276 276
Misses 918 918 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mtitov
left a comment
There was a problem hiding this comment.
overall looks good, just have a comment regarding stacked RU metrics
|
@mtitov : ping |
I am a bit annoyed by this, and allow me to explain one last time.
The plots thrown out by
radical-analytics-inspectare supposed to be used to obtain a quick overview over the details of an RP run. As such they show events, durations etc in quite some detail and high resolution, and with labels which only make sense if one know RP.Those plots have been repeatedly misunderstood as publication quality plots. They are not that. If you want publication plots, write an RA script to produce them, possibly basing those scripts on the
inspectscripts. Remember:radical.analyticsis module you can use to write RCT analysis plots - it is not a tool to create specific plots! Flexibility is its strength!Oh well - now the plots have less details, less precise labels, but look 'publication ready'. Please understand that I do not intent to become the person who creates all publication plots, and specifically don't intent to alter these scripts further if the plots are unfit for this this publication or that (which they, without a doubt, will be as each publication has a very different focus). If you need other plots, create your own scripts for those! It's easy!!