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

Correctly handle 2D/1D custom medium when exporting to GDS #1247

Merged
merged 1 commit into from
Nov 13, 2023

Conversation

lucas-flexcompute
Copy link
Collaborator

The 0D case is also handled just to avoid an obscure error.

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 @lucas-flexcompute, just a minor comment suggesting some more explanation so it's more clear what this fixes but otherwise looks good!

scale = max(b - a for a, b in zip(bb_min, bb_max))
for coord in (eps.x, eps.y, eps.z):
if len(coord) > 1:
scale = min(scale, np.diff(coord).min())
Copy link
Collaborator

Choose a reason for hiding this comment

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

what exactly does this do? My reading is that for the scale it takes the minimum of

  • the custom medium bounding box size
  • the custom medium pixel size?

What edge case does this fix? I guess before were custom medium not being resolved properly with the provided scale?

Maybe a comment here and also adding more detail in the changelog could be helpful down the line.

- Properly take pixel size into account in the length scale when converting a custom medium structure to GDS.

or something.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The pixel size was already taken into account before. The scale is the minimal distance between custom medium points with respect to each direction (x, y, z). The problem was that, if any of the directions had a single point, np.diff returned an empty array and the min function would raise an error. Now we initialize the scale with the largest possible value (from the bounding box) and, for each direction with at least 2 coordinates, keep only the minimum. Does it make sense to you? If so, I'll add those comments.

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh I see, yea if you could just add a comment in the source and maybe make the changelog item more specific to that effect, it would help a lot, thanks for explaining.

- Properly handle length sale in exporting custom medium structures to GDS when the geometry size is very small along some dimensions.

or something

The 0D case is also handled just to avoid an obscure error.

Signed-off-by: Lucas Heitzmann Gabrielli <lucas@flexcompute.com>
@lucas-flexcompute lucas-flexcompute merged commit 52e48ea into pre/2.5 Nov 13, 2023
14 checks passed
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.

None yet

2 participants