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

added plot_units to simulation and scene classes #1811

Merged
merged 1 commit into from
Jul 11, 2024

Conversation

dmarek-flex
Copy link
Contributor

@dmarek-flex dmarek-flex commented Jul 9, 2024

While working in the RF world I have been manually modifying plots so that the axes look good. But it is probably best to add that ability directly to tidy3d. I added an optional parameter to the Simulation and Scene classes that will automatically set the axes labels and title, along with a proper scaling of the tick labels.

Here is an example without setting plot_units:

Screenshot from 2024-07-09 14-28-45

And here when using "mm":

Screenshot from 2024-07-09 14-28-27

@dmarek-flex dmarek-flex force-pushed the dmarek/plot_with_units branch 2 times, most recently from 06945c9 to 8f7469c Compare July 9, 2024 19:40
Copy link
Collaborator

@tylerflex tylerflex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @dmarek-flex! This will definitely be nice to have

@dmarek-flex dmarek-flex force-pushed the dmarek/plot_with_units branch 3 times, most recently from 5675d3b to 24bd077 Compare July 10, 2024 15:33
@dmarek-flex dmarek-flex force-pushed the dmarek/plot_with_units branch from 24bd077 to e38eb20 Compare July 10, 2024 15:37
@dmarek-flex
Copy link
Contributor Author

@tylerflex any idea why that set of tests failed?

@tylerflex
Copy link
Collaborator

Probably something doesn't like the μ.

@tylerflex
Copy link
Collaborator

potentially related to the regex? https://github.com/flexcompute/tidy3d/blob/develop/scripts/make_script.py#L69

@dmarek-flex
Copy link
Contributor Author

@yaugenst-flex the more the merrier. I thought maybe you had changed make_script.py recently, but I think that was just the formatting commit.

Copy link
Collaborator

@weiliangjin2021 weiliangjin2021 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very helpful for now. Hopefully later we can set global units for time, length, etc.

Copy link
Collaborator

@yaugenst-flex yaugenst-flex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Yeah make_script.py got formatted and I did some minor refactoring, didn't touch the actual functionality. There is currently a problem with generating scripts from simulations that contain DataArrays, but that's a bit tricky to sort out.

@dmarek-flex dmarek-flex force-pushed the dmarek/plot_with_units branch from 57ad93d to 8dcaf4c Compare July 11, 2024 13:07
@tylerflex tylerflex self-requested a review July 11, 2024 13:16
@dmarek-flex dmarek-flex merged commit 4618626 into pre/2.8 Jul 11, 2024
16 checks passed
@dmarek-flex dmarek-flex deleted the dmarek/plot_with_units branch July 11, 2024 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants