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

Tyler/inf2 #242

Merged
merged 9 commits into from
Mar 6, 2022
Merged

Tyler/inf2 #242

merged 9 commits into from
Mar 6, 2022

Conversation

tylerflex
Copy link
Collaborator

Before, we made td.inf = np.inf and it worked, except for in Simulation. _filter_structures_plane() where it has to check intersections for the polygons against each other.

To fix, I added a way to evaluate a shapely.geometry.Polygon containing "inf" vertices.

The function looks at each vertex of the polygon and creates a copy where any infinite vertices are replaced by a signed td.constants.LARGE_NUMBER.

Then this is inserted into the Simulation. _filter_structures_plane() to avoid any problems in the plotting.

Might need to check it works for Circles as well.

@tylerflex
Copy link
Collaborator Author

tylerflex commented Mar 4, 2022

Alright, this should work but doesn't.

Seems there's some errors in the assert in discretize_ind.

But I can't reproduce, it only happens on the backend, it seems.

One notebook that triggers it is GratingCoupler.ipynb.

@tylerflex
Copy link
Collaborator Author

@momchil-flex this is ready for testing.

  • Plotting now supports objects with td.inf size.
  • PolySlab.slab_bounds is inf-aware.
  • Box.from_bounds is inf-aware.
  • center and radius are prohibited from being inf, to avoid inconsistent behavior.
    tests pass, but we should maybe run backend tests with this branch too, to catch runtime errors.

@momchil-flex momchil-flex merged commit 268f21c into develop Mar 6, 2022
@momchil-flex momchil-flex deleted the tyler/inf2 branch March 17, 2022 18:07
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.

2 participants