fix(util): plot_shmoo.py is broken on modern matplotlib and mis-wires its ticks#3
Open
seam0814 wants to merge 1 commit into
Open
Conversation
… its ticks
`util/plot_shmoo.py` cannot currently render a Shmoo plot in any
environment with matplotlib >= 3.5. Three independent issues were
preventing it from working as documented.
## 1. Tick formatters use the old 1-arg lambda signature
ax.xaxis.set_major_formatter(lambda val: f'{val:.0f}')
ax.yaxis.set_major_formatter(lambda val: f'{val:1.2f}')
Matplotlib 3.5+ calls callable formatters as `fmt(value, pos)`. The
1-arg lambdas raise `TypeError: <lambda>() takes 1 positional
argument but 2 were given` during the first `savefig`, before any
PNG is produced. Reproduced with matplotlib 3.10 on Python 3.12.
Fixed by widening the signature to `lambda val, _pos: ...`.
## 2. Tick locations were assigned to the wrong axis
The pcolormesh call is `ax.pcolormesh(ydata, xdata, bar_data)` with
`xdata = data['vs_v']` (voltages) and `ydata = data['fs_mhz']`
(frequencies). Per matplotlib's `pcolormesh(X, Y, C)` contract, this
means the plot's X axis is frequency and the Y axis is voltage — and
the existing `set_xlabel('Frequency (MHz)')` / `set_ylabel('Core
Voltage (V)')` confirms that intent.
However, the surrounding tick wiring was:
xticks = xdata[::5] # voltages (0.7..1.2 V)
yticks = ydata[::6] # frequencies (40..200 MHz)
plt.xticks(xticks, rotation=0) # voltage values placed on the freq axis
ax.set_yticks(yticks) # frequency values placed on the volt axis
Voltage tick positions (0.7..1.2) fall completely outside the
frequency axis range (~40..200), and vice versa, so even after the
formatter bug is fixed the axes come out with no usable tick marks.
Renamed the local variables to `freq_mhz` / `volt_v` so the tick
wiring cannot drift from the pcolormesh argument order, and changed
the pcolormesh call to the explicit `(freq_mhz, volt_v, bar_data)`
form. `bar_data` already has shape `(len(volt_v), len(freq_mhz))`, so
no data reshaping was needed.
## 3. CLI cannot express "no measurement" without crashing
The header comment documents calling the script with positional
placeholders for the optional `pmeas`/`psupply`/`pchan`/`cmeas`/
`citer`/`ops` arguments. CLI invocations cannot pass real `None`, so
those slots arrive as the literal string `"None"`, and the downstream
`if pmeas is not None` checks are always true — `generate_data` then
crashes with `KeyError: 'None'` while indexing `run["None"]`.
Conversely, calling the script with no optional args (just `out_file
runs_path`) makes `genargs[6]`, `genargs[4]`, `genargs[1]` raise
`IndexError` because the original `main()` body indexes positions
that may not exist.
Fixed both cases:
genargs = tuple(None if g in ('None', '') else g for g in genargs)
...
if len(genargs) > 6 and genargs[6] is not None: ...
so both `python plot_shmoo.py out.png runs.json` and `python
plot_shmoo.py out.png runs.json None None None None None None` now
produce a correct correctness-only Shmoo plot.
## Verification
Synthetic runs JSON with Vmin(F) = 0.6 + 0.0025*F over a 26x17
(voltage × frequency) sweep produces the expected diagonal Vmin
curve. Both the no-args and all-None-placeholder invocations now
succeed and write a valid PNG; the original code crashes on both.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
`util/plot_shmoo.py` cannot currently render a Shmoo plot in any
environment with `matplotlib >= 3.5`. Three independent issues prevent
it from working as documented; this PR fixes all three with the
minimum surgery required.
The util produces correctness-only and power/energy/efficiency-coloured
Shmoo plots from `parse_runs.py` output, so it's the primary published
characterization artefact for DUTCTL-driven silicon evaluation. Worth
having it actually run.
Issue 1 — tick formatters use the old 1-arg lambda signature
```python
ax.xaxis.set_major_formatter(lambda val: f'{val:.0f}')
ax.yaxis.set_major_formatter(lambda val: f'{val:1.2f}')
```
Matplotlib 3.5+ calls callable formatters as `fmt(value, pos)`. The
1-arg lambdas raise
```
TypeError: () takes 1 positional argument but 2 were given
```
during the first `savefig`, so no PNG is ever produced. Verified
with matplotlib 3.10 on Python 3.12.
Fixed by widening the signature to `lambda val, _pos: …`.
Issue 2 — tick locations were assigned to the wrong axis
The pcolormesh call is
```python
xdata = data['vs_v'] # voltages
ydata = data['fs_mhz'] # frequencies
…
c = ax.pcolormesh(ydata, xdata, bar_data, …)
```
Per matplotlib's `pcolormesh(X, Y, C)` contract, this means the plot's
X axis is frequency and the Y axis is voltage — and the existing
`set_xlabel('Frequency (MHz)')` / `set_ylabel('Core Voltage (V)')`
confirms that intent.
However the surrounding tick wiring was
```python
xticks = xdata[::5] # voltages 0.7..1.2 V
yticks = ydata[::6] # frequencies 40..200 MHz
plt.xticks(xticks, …) # voltage VALUES placed on the freq axis
ax.set_yticks(yticks) # frequency values placed on the volt axis
```
The voltage tick positions (0.7..1.2) fall completely outside the
frequency axis range, and vice versa, so even after the formatter bug
is fixed the axes come out with no usable tick marks at all.
Renamed the local variables to `freq_mhz` / `volt_v` so the tick
wiring cannot drift from the pcolormesh argument order, and changed
the pcolormesh call to the explicit `(freq_mhz, volt_v, bar_data)`
form. `bar_data` already has shape `(len(volt_v), len(freq_mhz))`,
so no data reshaping was needed.
Issue 3 — CLI cannot express "no measurement" without crashing
The file header documents calling the script with positional
placeholders for the optional `pmeas` / `psupply` / `pchan` /
`cmeas` / `citer` / `ops` arguments. CLI invocations cannot pass real
`None`, so those slots arrive as the literal string `"None"`, and the
downstream `if pmeas is not None` checks are always true.
`generate_data` then crashes with `KeyError: 'None'` while indexing
`run["None"]`.
Conversely, calling the script with no optional args (just `out_file
runs_path`) makes `genargs[6]`, `genargs[4]`, `genargs[1]` raise
`IndexError` because the original `main()` body indexes positions
that may not exist.
Fixed both cases by normalising `"None"` / `""` to Python `None` and
guarding the optional-arg checks with a length test:
```python
genargs = tuple(None if g in ('None', '') else g for g in genargs)
…
if len(genargs) > 6 and genargs[6] is not None: …
```
Both `python plot_shmoo.py out.png runs.json` and `python
plot_shmoo.py out.png runs.json None None None None None None` now
produce a correct correctness-only Shmoo plot.
Verification
A synthetic runs JSON with `Vmin(F) = 0.6 + 0.0025·F` over a 26×17
(voltage × frequency) sweep produces the expected diagonal Vmin
curve. Both the no-args and all-None-placeholder invocations now
succeed and write a valid PNG; the original code crashes on both.
Happy to add an example image to the PR thread if useful.
Scope
Single file change, no public API change. Only `util/plot_shmoo.py`
is touched.