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

Fix mzi notebook and batch #255

Merged
merged 5 commits into from Nov 30, 2023
Merged

Fix mzi notebook and batch #255

merged 5 commits into from Nov 30, 2023

Conversation

joamatab
Copy link
Contributor

@joamatab joamatab commented Nov 30, 2023

  • fix MZI notebook, thank you @lucas-flexcompute
  • for multithreaded batch submission create a new folder with the hash of the Tidy3dComponentModeller to avoid multibatch colisions

fixes flexcompute/tidy3d#1276

@yaugenst
@tylerflex

@tylerflex
Copy link

@joamatab note that after flexcompute/tidy3d#1259 is released officially, I think (?) you won't need to do the hash workaround. but glad you found a way around it for now.

@joamatab
Copy link
Contributor Author

Sounds good Tyler, let me know when make a pre-release so that we can start using it

@tylerflex
Copy link

2.5.0rc3 was just released a few minutes ago! so you can try that out

@@ -498,6 +499,7 @@ def write_sparameters(
pad_z_outer=pad_z_outer,
dilation=dilation,
)
path_dir = str(pathlib.Path(dirpath) / str(hash(c)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will hash to different values on different runs I believe? If we use the fix from flexcompute/tidy3d#1259 then we should call c._hash_self() here instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it has the same value on different runs

Copy link
Collaborator

Choose a reason for hiding this comment

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

are you sure? across different interpreter instances?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the hash is coming from Tidy3D component modeller

i run with different interpreter instances and was working well

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok perfect then 👍

Choose a reason for hiding this comment

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

@joamatab the problem is that hash() gets a new random seed with different interpreter instances. which is why we had to implement the _hash_self(). I'm actually surprised it gave the same results for you, I wouldn't have expected that. Might want to double check just to save yourself some headache later.

Copy link
Contributor Author

@joamatab joamatab Nov 30, 2023

Choose a reason for hiding this comment

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

Hi Tyler, yes, but Tidy3DComponent is not your modeler but a new object that inherits from the LayeredComponentBase that Yannick wrote

class Tidy3DComponent(LayeredComponentBase):
    """
    Represents a 3D component in the Tidy3D simulation environment.

    Attributes:
        material_mapping (dict[str, Tidy3DMedium]): A mapping of material names to Tidy3DMedium instances.
        extend_ports (NonNegativeFloat): The extension length for ports.
        port_offset (float): The offset for ports.
        pad_xy_inner (NonNegativeFloat): The inner padding in the xy-plane.
        pad_xy_outer (NonNegativeFloat): The outer padding in the xy-plane.
        pad_z_inner (float): The inner padding in the z-direction.
        pad_z_outer (NonNegativeFloat): The outer padding in the z-direction.
        dilation (float): Dilation of the polygon in the base by shifting each edge along its
            normal outwards direction by a distance;
            a negative value corresponds to erosion. Defaults to zero.
    """

    material_mapping: dict[str, Tidy3DMedium] = material_name_to_medium
    extend_ports: NonNegativeFloat = 0.5
    port_offset: float = 0.2
    pad_xy_inner: NonNegativeFloat = 2.0
    pad_xy_outer: NonNegativeFloat = 2.0
    pad_z_inner: float = 0.0
    pad_z_outer: NonNegativeFloat = 0.0
    dilation: float = 0.0

Choose a reason for hiding this comment

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

oh. my fault, please disregard

gplugins/tidy3d/component.py Outdated Show resolved Hide resolved
@joamatab joamatab merged commit f5adaa1 into main Nov 30, 2023
13 checks passed
@joamatab joamatab deleted the fix_mzi_notebook_and_batch branch November 30, 2023 23:02
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.

threading issues
4 participants