XY transposable sim.plot() plots (issue1072)

[jewettaijfc]

This PR addresses he feature request from issue #1072 (adding the ability to swap the horizontal and vertical axes of a simulation plot).

This includes:

• transposing all of the coordinate geometry
• swapping the X and Y axis labels

(I'm not yet sure whether I should swap the horizontal and vertical plot limits, because it might confuse the user. Will ask for feedback regarding this detail.)

---

UPDATE: There is an alternative version of this PR, (PR2544).

[jewettaijfc]

Although the basic functionality works (for sim.plot() and sim.plot_grid()), this pull-request is not yet ready for review. (I was hoping to make more progress today.)

As Tyler mentioned, there are a bunch of other "plot" functions in the code.

I'm going through them to find the ones where I should add a "transpose" argument.

[tylerflex]

Let's limit the scope to Simulation.plot(), Simulation.plot_eps(), and any sub-functions that they call where we will need to pass transpose in. (so for example, dont worry about plotting data outputs or HeatSimulation, etc)

and yes I think plot limits should also be transposed. You'll see there are some functions that compute vlims and hlims, I think it should be possible to pass transpose through to those and handle it.

Thanks!

[jewettaijfc]

Note to self: At a minimum, I still need to add the transpose argument to:

• Geometry.plot(). (Note: That function just invokes plot_shape(), which I already modified.)
• Source.plot()
• Monitor.plot()
• Geometry._plot_arrow()

I still need to test:

• The grid points are also transposed.
• Arrow directions are transposed

[jewettaijfc]

Let's limit the scope to Simulation.plot(), Simulation.plot_eps(), and any sub-functions that they call where we will need to pass transpose in. (so for example, dont worry about plotting data outputs or HeatSimulation, etc)

Thanks Tyler!

and yes I think plot limits should also be transposed. You'll see there are some functions that compute vlims and hlims, I think it should be possible to pass transpose through to those and handle it.

To clarify, here's a concrete example, plotting a simulation in the XY plane:

ax = sim.plot(z=0, ax=ax, hlim=[-4,4], vlim=[-8,8])

It will look like a vertical brick because the vertical limits are bigger than the horizontal limits.

Now suppose, we set transpose=True:

ax = sim.plot(z=0, ax=ax, hlim=[-4,4], vlim=[-8,8], transpose=True)

In that case, the entire plot will indeed be transposed. (It will look like a brick laid horizontally.) But it also means that the vertical range of the plot will be controlled by hlim (=[-4,4]). (And, similarly, the horizontal range of the plot will be controlled by vlim (=[-8,8])). Is that what you want?

[tylerflex]

oh I see what you mean. I think for vlim and hlim submitted by user, they should not be transposed. eg. it should always just correspond to the vertical and horizontal limits of the output (transposed or not)

However, when these values are computed internally (ie when not supplied by user), let's take the transpose into account.

Does that seem reasonable?

[jewettaijfc]

oh I see what you mean. I think for vlim and hlim submitted by user, they should not be transposed. eg. it should always just correspond to the vertical and horizontal limits of the output (transposed or not)

However, when these values are computed internally (ie when not supplied by user), let's take the transpose into account.

Does that seem reasonable?

Yeah! I'll take a look at that. Thanks!

Update:

• Working on rotating the grid points. (It's not working yet, but I haven't tried everything).
• In geometry/base/, I am updating plot_sources(), and plot_monitors(), plot_pml(), plot_lumped_elements(), plot_boundaries(). (These are all referenced by Simulation.plot()).
• Reading about plot_symmetries(), and deciding how to update it.

[jewettaijfc]

(I just committed the changes that I made to this branch today in order to backup this code. It runs without crashing, but it has some weird quirks, which will require debugging.)

Hi Tyler: I don't plan on continuing with this branch today. As I mentioned on slack, I'm going to start a new PR that uses a different approach. (If we decide that approach ultimately works better, we can abandon this PR.)

[jewettaijfc]

I already said this to Tyler in our Slack-chat, but Dario (if you are reading this), here's the reason I am deprecating this PR (for now): There were just too many places in the code where I had to add something like this:

if transpose:
    do this custom thing
else:
    do the regular thing

Many different functions and types of plots needed a different "custom thing" . I just wasn't sure in every case that the "custom thing" I was adding was correct. Testing each custom thing I added was hard.

I'm about 80% done with creating a newPR which uses a different approach. Going to bed.

[tylerflex]

can use raise NotImplementedError(...) here

[jewettaijfc]

Thanks. (If we decide to continue with this PR, I'll need to come back and fix this part of the code.)

[tylerflex]

@jewettaijfc actually I think this PR is basically in line with what I expect.

The only thing I'm a little confused about is why the new

Geometry.transpose_axis_info

vs passing transpose to

Box.add_ax_labels_and_title

and handling it there?

otherwise I think the if/else complications are simply unavoidable due to the nature of this problem. Although there were a few areas I saw where it could probably be cleaned up slightly for example

           if transpose:
                ax.plot(ys, xs, color=plot_params.facecolor, linewidth=plot_params.linewidth)
            else:
                ax.plot(xs, ys, color=plot_params.facecolor, linewidth=plot_params.linewidth)

could be

           if transpose:
                xs, ys = ys, xs
           ax.plot(xs, xs, color=plot_params.facecolor, linewidth=plot_params.linewidth)

[jewettaijfc]

           if transpose:
                xs, ys = ys, xs
           ax.plot(xs, xs, color=plot_params.facecolor, linewidth=plot_params.linewidth)

Good suggestion!

[jewettaijfc]

The only thing I'm a little confused about is why the new

Geometry.transpose_axis_info

vs passing transpose to

Box.add_ax_labels_and_title

and handling it there?

Ahh. Good question. In my first few commits, I added this line at the end of my plot_sim() and plot_grid() functions to swap the axis labels. Later, I realized I need to update some of the other plot_ functions as well. Remembering which functions I need to modify was a cognitive burden. So it was just easier to apply the same pattern to all of the plot functions:

That is: Whenever self.add_ax_labels_and_title() is invoked, follow it with Geometry.transpose_axis_info().

I can stop doing that in Box.add_ax_labels_and_title() and other low-level functions, if you prefer.

(The other PR I'm working does not use this pattern.)

[jewettaijfc]

After reading your comment, I also noticed that one of the add_ax_labels_and_title() functions mistakenly has transpose as an argument. Thanks for catching that! I will take that out.

[jewettaijfc]

For the record, the other PR is #2544.

[jewettaijfc]

The other PR (#2544 ) is working well and is much further along. I think I will close this PR for now so that people don't get confused. We can always reopen it later if we want to.