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

Automatic Waveguide SBend #179

Merged
merged 6 commits into from
Aug 26, 2022
Merged

Automatic Waveguide SBend #179

merged 6 commits into from
Aug 26, 2022

Conversation

jevillegasd
Copy link
Contributor

Inserts waveguide bends #153 as part of the guiding line that generates the waveguide. The SBend takes precedence over the creation of normal bends where it's possible to install them, but they are only placed provided a normal bend doesn't fit. It uses the Bezier Parallel function from SiEPIC.utils.geometry.

Here are some images of how the Bends look.
image

Tested it against EBeam, Athabasca, AMF, and others and it seems to be working fine in all.

Concerns:

  • The warning for radius clearance appears even if an SBend can be placed on the site.
  • I had to do a weird import for the optimizer to work on my end. I don't know what functions, other than the optimal bezier one, are using it so I couldn't track if it affected anything else.

Inserts waveguide bends SiEPIC#153  as part of the guiding line that generates the waveguide. The SBend takes precedence over the creation of normal bends where it's possible to install them, but they are only placed provided a normal bend doesn't fit. It uses the Bezier Parallel function from SiEPIC.utils.geometry, but I had to do a weird import for the optimizer for it to work on my end.
@jevillegasd
Copy link
Contributor Author

Found a bug when the sbend starts too close from a previous normal bend (case that should not try to install an SBend I suppose, giving precendece to whoever is the first in the list of points). I am fixing this and will send a new commit

image

@mustafacc
Copy link
Member

Wow @jevillegasd - you work fast.

This is quite nice - I've been testing it on a few PDKs, and so far, so good.

Some odd behaviours I'm seeing on my end (testing on EBeam PDK):

  • The S-bend seems to only instantiate on the second inflection point but not the first. Looking at your samples, it seems like that error is not occurring on your end. I'm debugging further.

image

  • Deleting the new waveguide PCell results in an internal error (note that this error only occurs when the above behaviour is present in the cell)
    image

I agree with adding a checkbox to support older waveguides. Can this property be used to catch the false "minimum radius" DFT flag? i.e., if s-bend is checked, and the minimum radius flag occurs at the S-bend, then it's a false positive. I think this can be sorted out in the future.

(FYI I'm holding off on merging this pull request until SiEPIC-Tools V0.3.93 is released next week - we're working on a stable version for the release of https://www.aimphotonics.com/dream-photonics-component-library)

Was checking the wrong distance for clearance before inserting the guide.
@jevillegasd
Copy link
Contributor Author

jevillegasd commented Aug 21, 2022

Hey @mustafacc , I was able to reproduce your error and fixed the it, it was a tiny mistake when checking if there was enough clearance for the bend.

The minimum error marking is still handled by the existing waveguide methods, if no s-bend is placed, then it tries to isnert normal bends and detects there if there are any minimum bend violations. Because of this there is a condition, when an sbend is too close to one of the ends, that gets detected as a minimum radius violation instead of a 'not enough clearance for sbend' , because well, no sbends violations are being handled. That's the situation you had in the image you posted (although still the method was checking for the wrong clearance).

About the internal error popping out I was not able to reproduce it, even before the fix I did.

  • I will keep checking if I can figure that out.
  • Will also update for the 'insert sbend' parameter and commit those changes.

With this change, the sbends are optional and defined by the waveguide XML file by adding the sbends parameter to the specification. That is adding to each waveguide component:
<sbends>y</sbends>

If the said element doesn't exist as a waveguide parameter, the flag 'sbends' in  layout_waveguide3 gets set as False and passed to the layout_waveguide2 method.
@jevillegasd
Copy link
Contributor Author

This last commit should do the job. It needs updating the WAVEGUIDE.XML file with the parameter <sbends>true</sbends> to allow inserting the sbends.

@mustafacc
Copy link
Member

Thanks @jevillegasd - I think I came across another case that's missed by the automated S-bend generation: S-bends seem to be generating nicely laterally. However, it seems like the vertical sections are missed. I can see from your screenshots that you are able to instantiate vertical S-bends without the issue I'm seeing:

image

Regarding S-bend option in waveguidex.xml: is it possible to set the default to be true unless the user overrides this argument? The main reason is to enable this feature for backwards compatibility without having to update all the various PDKs waveguide.xml with " true" option.

@jevillegasd
Copy link
Contributor Author

Hey @mustafacc, thanks for double checking. trying to replicate your error I found out it's worse than I thought! Actually right now the SBend placement is working only drawing from bottom to top or left to right waveguides. I'll debug and update the commit.

@jevillegasd
Copy link
Contributor Author

jevillegasd commented Aug 22, 2022

Hey @mustafacc . I've corrected the issue, just needed to include a condition to mirror the waveguide points upon creation. These are the test cases that I used, and it worked well in all of them. Please let me know if you can give it a look and if it works on your end.

image

The 'circle' paths are clockwise and counter-clockwise and with bends towards the outside and the inside.

@mustafacc
Copy link
Member

Thanks a lot @jevillegasd , I've tested on additional test cases and PDKs and the automated S-bend works on all the cases so far.

@mustafacc mustafacc merged commit efebfbb into SiEPIC:master Aug 26, 2022
@jevillegasd jevillegasd deleted the SBend branch August 30, 2022 14:10
@jevillegasd jevillegasd mentioned this pull request Jan 17, 2023
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