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.

Sign up for GitHub

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

Jump to bottom

Rename fiber ports #2700

Merged
merged 2 commits into from Apr 24, 2024
Merged

Rename fiber ports #2700

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f720736
use vertical_{polarization} for port names
joamatab Apr 24, 2024
9883c6a
rename edge coupler ports
joamatab Apr 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 14 additions & 2 deletions gdsfactory/components/edge_coupler_array.py
View file
Open in desktop
Expand Up @@ -18,8 +18,20 @@
Float2,
)

edge_coupler_silicon = partial(taper, width2=0.2, length=100, with_two_ports=True)
edge_coupler_silicon_2 = partial(taper, width2=0.2, length=130, with_two_ports=True)
edge_coupler_silicon = partial(
taper,
width2=0.2,
length=100,
with_two_ports=True,
port_order_types=("optical", "edge_te"),
Comment on lines +21 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (code_clarification): Consider documenting the addition of 'port_order_types' parameter.

Adding new parameters can affect how the function is used or its outputs. It's important to update the documentation to reflect these changes and provide guidance on how to use the new parameter.

Comment on lines +21 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (testing): Missing test for updated port_order_types parameter.

The changes to the 'port_order_types' parameter in 'edge_coupler_silicon' and 'edge_coupler_silicon_2' should be covered by unit tests to ensure that the new naming convention is correctly implemented and does not affect other functionalities.

)
edge_coupler_silicon_2 = partial(
taper,
width2=0.2,
length=130,
with_two_ports=True,
port_order_types=("optical", "edge_te"),
)


@gf.cell
Expand Down
6 changes: 3 additions & 3 deletions gdsfactory/components/grating_coupler_elliptical.py
View file
Open in desktop
Expand Up @@ -100,7 +100,6 @@ def grating_taper_points(

@gf.cell
def grating_coupler_elliptical(
polarization: str = "te",
taper_length: float = 16.6,
taper_angle: float = 40.0,
wavelength: float = 1.554,
Expand All @@ -115,12 +114,12 @@ def grating_coupler_elliptical(
slab_offset: float = 2.0,
spiked: bool = True,
cross_section: CrossSectionSpec = "xs_sc",
polarization: str = "te",
**kwargs,
) -> Component:
r"""Grating coupler with parametrization based on Lumerical FDTD simulation.

Args:
polarization: te or tm.
taper_length: taper length from input.
taper_angle: grating flare angle.
wavelength: grating transmission central wavelength (um).
Expand All @@ -135,6 +134,7 @@ def grating_coupler_elliptical(
slab_offset: in um.
spiked: grating teeth have sharp spikes to avoid non-manhattan drc errors.
cross_section: specification (CrossSection, string or dict).
polarization: te or tm.
kwargs: cross_section settings.

.. code::
Expand Down Expand Up @@ -259,7 +259,7 @@ def grating_coupler_elliptical(
width=10,
orientation=0,
layer=layer,
port_type="optical",
port_type=f"optical_{polarization}",
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (testing): Missing test for dynamic port_type based on polarization.

The introduction of a dynamic 'port_type' based on the 'polarization' parameter should be validated through tests to ensure that it behaves as expected across different inputs.

Suggested change
port_type=f"optical_{polarization}",
import pytest
@pytest.mark.parametrize("polarization", ["TE", "TM", "random"])
def test_port_type(polarization):
expected_port_type = f"optical_{polarization}"
# Assuming a function `create_component` that uses the `port_type` logic
component = create_component(polarization=polarization)
assert component.port_type == expected_port_type

)

xs.add_bbox(c)
Expand Down
2 changes: 1 addition & 1 deletion gdsfactory/components/grating_coupler_elliptical_arbitrary.py
View file
Open in desktop
Expand Up @@ -164,7 +164,7 @@ def grating_coupler_elliptical_arbitrary(
width=10,
orientation=0,
layer=xs.layer,
port_type="optical",
port_type=f"vertical_{polarization}",
)
xs.add_bbox(c)
return c
Expand Down
2 changes: 1 addition & 1 deletion gdsfactory/components/grating_coupler_elliptical_trenches.py
View file
Open in desktop
Expand Up @@ -133,7 +133,7 @@ def grating_coupler_elliptical_trenches(
width=10,
orientation=0,
layer=layer,
port_type="optical",
port_type=f"optical_{polarization}",
)
return c

Expand Down
2 changes: 1 addition & 1 deletion gdsfactory/components/grating_coupler_rectangular.py
View file
Open in desktop
Expand Up @@ -141,7 +141,7 @@ def grating_coupler_rectangular(
xport = np.round((x0 + cgrating.x) / 2, 3)
c.add_port(
name="o2",
port_type="optical",
port_type=f"vertical_{polarization}",
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion (testing): Ensure consistency in port naming convention.

The test should verify that the new 'vertical_{polarization}' naming is consistent across all components and does not introduce discrepancies.

Suggested change
port_type=f"vertical_{polarization}",
import pytest
def test_vertical_polarization_naming_consistency():
# Assuming `get_port_type` is a method that retrieves the port type of a component
# and `expected_polarization` is the polarization we expect to be part of the port type name.
component = create_grating_coupler() # Function to create a grating coupler component
port_type = component.get_port_type()
expected_polarization = "vertical_TE" # Example polarization, adjust as necessary
assert expected_polarization in port_type, f"Port type '{port_type}' does not include expected polarization '{expected_polarization}'"

center=(xport, 0),
orientation=0,
width=width_grating,
Expand Down
2 changes: 1 addition & 1 deletion gdsfactory/components/grating_coupler_rectangular_arbitrary.py
View file
Open in desktop
Expand Up @@ -159,7 +159,7 @@ def grating_coupler_rectangular_arbitrary(
xport = np.round((xi + length_taper) / 2, 3)
c.add_port(
name="o2",
port_type="optical",
port_type=f"vertical_{polarization}",
center=(xport, 0),
orientation=0,
width=width_grating,
Expand Down
2 changes: 1 addition & 1 deletion gdsfactory/components/grating_coupler_rectangular_arbitrary_slab.py
View file
Open in desktop
Expand Up @@ -128,7 +128,7 @@ def grating_coupler_rectangular_arbitrary_slab(

c.add_port(
name="o2",
port_type="optical",
port_type=f"vertical_{polarization}",
center=(xport, 0),
orientation=0,
width=width_grating,
Expand Down
3 changes: 3 additions & 0 deletions test-data-regression/test_settings_edge_coupler_array_.yml
View file
Open in desktop
Expand Up @@ -12,6 +12,9 @@ settings:
module: gdsfactory.components.taper
settings:
length: 100
port_order_types:
- optical
- edge_te
width2: 0.2
with_two_ports: true
n: 5
Expand Down
3 changes: 3 additions & 0 deletions test-data-regression/test_settings_edge_coupler_array_with_loopback_.yml
View file
Open in desktop
Expand Up @@ -13,6 +13,9 @@ settings:
module: gdsfactory.components.taper
settings:
length: 100
port_order_types:
- optical
- edge_te
width2: 0.2
with_two_ports: true
extension_length: 1.0
Expand Down
4 changes: 2 additions & 2 deletions test-data-regression/test_settings_edge_coupler_silicon_.yml
View file
Open in desktop
Expand Up @@ -4,7 +4,7 @@ info:
width1: 0.5
width2: 0.2
module: gdsfactory.components.taper
name: taper_length100_width20p2
name: taper_680c9faf
settings:
cross_section: xs_sc
length: 100
Expand All @@ -14,7 +14,7 @@ settings:
- o2
port_order_types:
- optical
- optical
- edge_te
width1: 0.5
width2: 0.2
with_two_ports: true