XY transposable sim.plot() plots (issue1072) VERSION 2

[jewettaijfc]

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

(Note: plot_field() is beyond the scope of this PR)

This PR is an alternative to PR#2537

I don't know which approach will work better in the long run. I will test both.

---

Motivation

I can think of 2 different strategies to transpose XY axes:

1. Wait until the very end, after the matplotlib axes object has been fully defined. Then swap the horizontal and vertical axes at display time.
2. Swap the geometry, limits, and axes-label information before sending them to matplotlib.

• The previous PR #2537 uses a combination of both strategies. It also tries to minimalize change to the existing code needed just to get Simulation.plot() to work.
• This PR uses the second strategy, and it updates all of the plot-related functions, not just Simulation.plot().

Specifically:

• I systematically added a transpose argument to all plot-related functions that previously accepted axis or x, y, z arguments. (Conceptually, these arguments all go together: they specify how to display a plane from a 3D space on the screen.)
• This includes pop_axis() and unpop_axis(), and the (plot-related) functions that invoke them ...all the way up to the top-level plot() functions.
• By default transpose=False everywhere. So unless someone overrides this by setting transpose=True, the code should behave as it did before.

It seems like I made a lot of complicated changes in this PR, but I really didn't. The only non-trivial changes to the code happen inside pop_axis() and unpop_axis() (see below). All of the higher-level functions merely propagate the transpose argument downwards until it is finally processed by either pop_axis() or unpop_axis(). This reduces the cognitive burden on the programmer: In each function, I no longer have to think hard about how to deal with the new transpose argument. I just forward the transpose argument downwards to any functions that ultimately invoke either pop_axis() or unpop_axis(). If we do this consistently, we hopefully shouldn't see surprising behavior later.

(UPDATE: These functions are now called pop_axis_and_swap() and unpop_axis_and_swap().)

---

For completeness, here are the new definitions of pop_axis_and_swap() and unpop_axis_and_swap():

    def pop_axis_and_swap(
        coord: tuple[Any, Any, Any],
        axis: int,
        transpose: bool = False,                         # <-- newly added
    ) -> tuple[Any, tuple[Any, Any]]:
        plane_vals = list(coord)
        axis_val = plane_vals.pop(axis)
        if transpose:                                    # <-- newly added
            plane_vals = [plane_vals[1], plane_vals[0]]  # <-- newly added
        return axis_val, tuple(plane_vals)

    def unpop_axis_and_swap(
        ax_coord: Any,
        plane_coords: tuple[Any, Any],
        axis: int,
        transpose: bool = False,              # <-- newly added
    ) -> tuple[Any, Any, Any]:
	coords = list(plane_coords)
        if transpose:                         # <-- newly added
            coords = [coords[1], coords[0]]   # <-- newly added
        coords.insert(axis, ax_coord)
        return tuple(coords)

That's all this PR does.

[jewettaijfc]

I don't want to add complexity to your code where it's not needed, @tylerflex ! The pop_axis() and unpop_axis() functions appear in many places. Let me know if I made too many modifications. I tried to avoid changing functions which are clearly not plot-related, even if they invoke pop_axis() or unpop_axis(). Specifically:

• I did not modify beam.py, boundary.py, field_projection.py, geometry/polyslab.py, geometry/utils2d.py, grid/*.py, lumped_element.py, medium.py, microwave/formulas/circuit_parameters.py, mode_solver.py, source/field.py, or any files outside of the components/ folder.
• I did not modify any functions that compute derivatives.
• I did not modify geometry/primitives.py, except for the various *intersections*() and *cross_section() functions (which are frequently needed for plotting).
• I did not modify monitor.py, except for the plot related functions.

[jewettaijfc]

This code has not yet been tested. It probably won't even run. I am beginning the (manual) testing process now.

[tylerflex]

we probably dont need to pass transpose to these gds functions, these are used to export the polygons to file.

[tylerflex]

hey @jewettaijfc , this is an interesting approach. I think it looks promising! I would have a few pieces of advice at this stage:

1. invest in a good solid test case. Dig into the test_viz.py and write some plot tests with transpose=True. Learn how to run the unit tests (should be in the developer guide) and that will save you a lot of time as you test this.
2. No need to pass transpose to any functions involving gds, eg to_gds. These are used when importing and exporting to GDSII files. https://www.flexcompute.com/tidy3d/examples/notebooks/GDSImport/
3. Pretty minor comment but might as well start thinking about the argument naming. I am pretty confident that a bool is the right data type for this option, but the naming I'm not sure about.
   a. in the plotting functions, we could also consider like swap_axes : bool = False?
   b. in the pop_axis and unpop_axis, maybe we consider something similar? or swap_planar_coords?
4. The only thing I'm still not sure about / hesitant about is whether we should be modifying these pop_axis and unpop_axis functions directly. I'm leaning towards thinking it's ok, but another way we could approach this to introduce new functions pop_axis_and_swap(coords, axis, swap_axes: bool) that simply calls pop_axis(coords, axis) and swaps the results if swap_axes is true.). Something to think about, but not sure if it improves anything or just complicates things.

Thanks for the effort on this! good luck on the testing.

[jewettaijfc]

hey @jewettaijfc , this is an interesting approach. I think it looks promising! I would have a few pieces of advice at this stage:

1. invest in a good solid test case. Dig into the test_viz.py and write some plot tests with transpose=True. Learn how to run the unit tests (should be in the developer guide) and that will save you a lot of time as you test this.

For sure. Regarding my work-flow, I don't put PRs up for review until I have some unit tests. It takes a little extra time to write them, but unit tests have saved my butt so many times.

2. No need to pass transpose to any functions involving gds, eg to_gds. These are used when importing and exporting to GDSII files. https://www.flexcompute.com/tidy3d/examples/notebooks/GDSImport/

Thanks! I'll fix that now.

3. Pretty minor comment but might as well start thinking about the argument naming. I am pretty confident that a bool is the right data type for this option, but the naming I'm not sure about.
   a. in the plotting functions, we could also consider like swap_axes : bool = False?
   b. in the pop_axis and unpop_axis, maybe we consider something similar? or swap_planar_coords?

I will let you decide. But I suggest we use the same name for all functions. (I like both swap_axes and transpose because the names are short compared to swap_panar_coords.)

4. The only thing I'm still not sure about / hesitant about is whether we should be modifying these pop_axis and unpop_axis functions directly. I'm leaning towards thinking it's ok, but another way we could approach this to introduce new functions pop_axis_and_swap(coords, axis, swap_axes: bool) that simply calls pop_axis(coords, axis) and swaps the results if swap_axes is true.). Something to think about, but not sure if it improves anything or just complicates things.

I was wondering about this too. I will look into that.

Thanks for the effort on this! good luck on the testing.

Thank you!

[jewettaijfc]

Currently:

• import tidy3d works without crashing
• Box.plot(), is working (with transpose=New)
• Scene.plot(), is working (with transpose=New), except when the scene contains Polyslabs (See below)

Here are some things fixed in this PR (which weren't working before in PR2537):

• Monitor.plot() is working (with transpose=True).
• Source.plot() is working (with transpose=True).
• plot_arrow() is working (with transpose=True).
• When hlim and vlim are unspecified (=None), the boundaries are correctly swapped when transpose=True. (This is what we want to happen.)
• Simulation.plot_grid() seems to be working (with transpose=True). This needs more testing...

Here are some things that are currently broken in this PR (which were working in PN2537):

• Transposing Polyslabobjects does not work yet. But I know why: I haven't made any changes to thegeometry/polyslab.py` file yet. I'll work on that tomorrow.

[jewettaijfc]

• PolySlabs are working with transpose=True. (I had to add a transpose argument to PolySlab._intersect_normal() and PolySlab._intersect_side().)
• I will test cylinders next.

NOTE: I left some print() statements in the code that I'm still using for (manual) debugging. I'll remove these soon.

[jewettaijfc]

• Cylinders are working with transpose=True

[jewettaijfc]

I'm done with manual testing.

I started writing unit tests.

[jewettaijfc]

Oops. I guess I wasn't done with manual testing.
I fixed a bug and (visually) verified that

• Simulation.plot_eps() is working (with both transpose=True and False).
• Now all of the functions in VisSimulation.ipynb are working (with both transpose=True and False).

[jewettaijfc]

• HeatChargeSimulation.plot_sources() is now working when swap_axes=True and false
• The HeatSolver.ipynb notebook is working now

[jewettaijfc]

• The HeatDissipation.ipynb notebook (from the tidy3d-notebooks examples) also seems to be working ...up until the point where I have to submit simulations. (I can't run the remaining portion of the .ipynb notebook because it looks like my free tidy3d account has expired. I'm working on fixing this.)

[tylerflex]

I think this is causing the error, this line should be put back

[tylerflex]

yup! just verified that it works after adding it back

[jewettaijfc]

Wow. Thank you! This would have taken me hours to figure out.

[tylerflex]

I'm worried this code will not work with autograd (the package I mentioned that handles our automatic differentiation).

I would suggest replacing with

vertices = vertices[:, ::-1]   # reverse the second axis

[jewettaijfc]

Thanks!
Other than pop_axis_and_swap() and unpop_axis_and_swap(), this is the only other place in the code (that I am aware of) containing non-trivial modifications. I need to write a unit-test for it.

[tylerflex]

good catch, this is probably more robust than what we had before.

[jewettaijfc]

I think my previous two commits broke all of the plot_field() functions.

EDIT 2025-6-10: I JUST REMOVED THOSE COMMITS FROM THIS PR. THEY NO LONGER APPEAR IN THE HISTORY

In retrospect, I'm not sure I should be modifying these functions. Most of the plot_field() functions already have a **sel_kwargs argument which seems to accomplish the same goal. (For example, here.) I worry that both **sel_kwargs and swap_axes in the same function might be redundant. But perhaps I am misunderstanding what **sel_kwargs was intended for.

• For consistency, I'd prefer to have only argument to specify planar axes (eg. either **sel_kwargs or swap_axes) that is shared by all the plots (eg. plot_sim() and plot_field()).
• I'll ask for guidance in tomorrow's TidyGrad Office Hours.

[jewettaijfc]

It sounds like plot_field() might be out of scope for this PR.
I'm starting a new PR that includes all of the changes to get plot_field() working.
I'll move the last two commits (no longer visible here) into that PR. (This will remove them from this PR.)

[jewettaijfc]

Minor issue:

A few days ago, commit de2d14d accidentally added a binary file to this branch (named results.prof_BRANCH=develop in the root directory). I removed it using git rm, but it's still in the git history and increasing the size of our user's downloads by 300kb.

---

Note to self: Later on, if we really want to purge that file from the history, we can use:

pip install git-filter-repo

cd ~/tidy3d

LARGE_BINARY_FILE_PATH="results.prof_BRANCH=develop"
git filter-repo --path "$LARGE_BINARY_FILE_PATH" --invert-paths

# (Note: The --path argument is interpreted relative to the repository 
#  root directory.  In this example, the file was in the root directory.)

Note: If you are not in the main branch, you probably have to use this command instead:

git filter-repo --path "$LARGE_BINARY_FILE" --invert-paths --force

Clean up:

git reflog expire --expire=now --all
git gc --prune=now

git push origin --force-with-lease --all
git push origin --force-with-lease --tags

I am not brave enough to try this yet.

[tylerflex]

what I would recommend is doing git rebase -i develop and squashing all of the commits to 1. we will do this before merging anyway. the file should be erased in this process.