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

Can't generate multipanel plot with PhasePlot #4489

Closed
gravertino opened this issue Jun 10, 2023 · 7 comments · Fixed by #4491
Closed

Can't generate multipanel plot with PhasePlot #4489

gravertino opened this issue Jun 10, 2023 · 7 comments · Fixed by #4491
Milestone

Comments

@gravertino
Copy link

Bug report

Bug summary

A multipanel plot with PhasePlot can not be generated normally following the guidance in the cookbook unless using annotate_title to add a title.

Code for reproduction

A minimum reproducible example is listed below. This example can not generate a mutlipanel phase plot normally (shown in Actual outcome ). However, if the line phs.annotate_title(title) hasn't been commented out, the plot will be created correctly (shown in Expected outcome ).

import matplotlib.pyplot as plt
from mpl_toolkits.axes_grid1 import AxesGrid
from yt.testing import fake_amr_ds
import yt

fig = plt.figure()
grid = AxesGrid(
fig,
111,
nrows_ncols=(2, 2),
axes_pad=0,
label_mode="L",
share_all=True,
cbar_location="right",
cbar_mode="single",
cbar_size="3%",
cbar_pad="0%",
aspect=True )
for i,title in enumerate(["995","1133","37373","37548"]):
    ds = fake_amr_ds()
    phs = yt.PhasePlot(ds,("gas","x"),("gas","y"),("gas","volume"),weight_field=None)
    # phs.annotate_title(title) # the multipanel plot can't be generated if this line has been commented out.
    plot = phs.plots[("gas","volume")]
    plot.figure = fig
    plot.axes = grid[i].axes
    if i == 0:
        plot.cax = grid.cbar_axes[i]
    phs.render()
plt.savefig("test.jpg")

Actual outcome

abnormal

Expected outcome

normal

Version Information

  • Operating System: WSL2 (Ubuntu-20.04)
  • Python Version: 3.8.10
  • yt version: 4.3.dev0
  • Other Libraries (if applicable): matplotlib: 3.6.2
@welcome
Copy link

welcome bot commented Jun 10, 2023

Hi, and welcome to yt! Thanks for opening your first issue. We have an issue template that helps us to gather relevant information to help diagnosing and fixing the issue.

@neutrinoceros
Copy link
Member

neutrinoceros commented Jun 10, 2023

wow, that's a weird one !
An interesting consequence of the current behaviour is that the title-less version of the multi-panel figure may be obtained by calling phs.annotate_title("").

Thank you for reporting and providing a nice, self-contained and minimal reproducer ! I'll look into it now.

@neutrinoceros
Copy link
Member

So, it's actually not something weird going on with annotate_title specifically: any plot annotation has a similar effect here. What they all have in common is that they invalidate the plot object: they force a new rendering next time the figure needs to be displayed.
In fact, replacing psh.annotate_title("") with phs._plot_valid = False also produces the correct result.
I also note that if I replace the phase plot with a slice plot, _plot_valid is False from the start (no annotation needed), so the end result is correct.
This seems to indicate that the attribute _plot_valid is improperly initialised to True in PhasePlot.

@neutrinoceros
Copy link
Member

Got a minimal fix in #4491

@neutrinoceros neutrinoceros added this to the 4.2.1 milestone Jun 10, 2023
@gravertino
Copy link
Author

Thank you for looking into this problem, and this explanation sounds very reasonable! But I still have a little question, why phs.render() didn't redraw itself as expected?

@neutrinoceros
Copy link
Member

That's a great question !
The render method calls a private, equivalent method _setup_plots. Looking how PhasePlot._setup_plots is implemented, we see that it intentionally does nothing when the _plot_valid attribute is True

def _setup_plots(self):
if self._plot_valid:
return

In fact, all other PlotContainer classes (like SlicePlot, ProjectionPlot ...) follow this pattern too.

You are right that it's also a bug that explicitly calling render() isn't doing anything. In my opinion it should never be no-op. I'll open another PR to fix that too

@gravertino
Copy link
Author

Thank you for your clear reply and now I understand what's happening here! Many thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants