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

arrow length based on simulation bounds #387

Merged
merged 1 commit into from
May 26, 2022
Merged

Conversation

tylerflex
Copy link
Collaborator

@tylerflex tylerflex commented May 26, 2022

source and monitor plotting accepts an optional sim_bounds argument.
If supplied, the arrow lengths are determined by the ARROW_LENGTH_FACTOR times the minimum length or width between the bounds on the plotting plane.

This avoids issues with using the axis limits (see #358), which might be temporarily resized for infinite monitor or source object, such as plane waves.

@tylerflex tylerflex linked an issue May 26, 2022 that may be closed by this pull request
Copy link
Contributor

@shashwat-sh shashwat-sh left a comment

Choose a reason for hiding this comment

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

I think it's a good solution. The idea of providing bounds works, but to a user it may not be obvious that the solution to this issue is to supply bounds. Maybe we can add an example of this in one of the visualization notebooks?

I'm wondering if there's a way to preempt the situation in an imperfect way but one that still works reasonably in the event that the user does not supply bounds. E.g., maybe there could be a default axis size (width, height) specified in viz.py (counting on the fact that most python plots will be in the ballpark of ~400 x ~400 pixels, for example?) and then using that to compute arrow length if bounds are not supplied AND size is inf.

Or maybe if the component detects its size is inf and bounds are not supplied while plotting arrows, a helpful warning could be thrown to suggest supplying bounds?

I don't think the above is critical though.

@tylerflex
Copy link
Collaborator Author

I think the only time this is an issue is if the following conditions are met:

  1. The source (or monitor) has infinite (or very large) bounds.
  2. The source (or monitor) is plotted from within a Simulation.plot_xxx() function.

If the user tries to do plane_wave.plot(z=0), it will just display the entire thing on an axis with bounds extending the full length of the (almost) infinite object. So it works without providing sim_bounds.

Basically, the way it's set up now, the user should never really use sim_bounds, it's more a way for us to tag the arrow plotting as happening within another plotting context and make sure that the correct size is specified.

@shashwat-sh
Copy link
Contributor

Ok, sounds good then, I don't have any more comments.

@tylerflex tylerflex merged commit a961d96 into develop May 26, 2022
@tylerflex tylerflex deleted the tyler/arrow_fix branch May 26, 2022 20:52
@tylerflex
Copy link
Collaborator Author

ok, thanks for looking it over

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.

Simulation plots in the source plane with arrows
2 participants