Skip to content

Conversation

@momchil-flex
Copy link
Collaborator

Nice example btw! :)

One of the checks was erroring for me, which is what this PR fixes (maybe numpy v2 vs. v1 issue? not sure at which point this has become a hard error).

More generally, was this AI generated?

    if not isinstance(field_data, td.FieldData):
        raise TypeError("field_data must be a tidy3d.FieldData.")
    if not isinstance(permittivity_data, td.PermittivityData):
        raise TypeError("permittivity_data must be a tidy3d.PermittivityData.")
    missing = [comp for comp in ("Ex", "Ey", "Ez") if comp not in dir(field_data)]
    if missing:
        raise KeyError(f"Missing field components in field_data: {missing}")
    if (
        np.any(permittivity_data.eps_xx != permittivity_data.eps_yy)
        or np.any(permittivity_data.eps_yy != permittivity_data.eps_zz)
    ):
        raise ValueError("Permittivity_data must be isotropic.")

AI likes to do all these defensive checks. Normally, we don't really do this in our notebooks, and it would be quite verbose to do so everywhere. I feel like it can just be removed?

Regarding the isotropic permittivity check though (which is what I'm fixing), I feel like there's even a potential issue: why do we need the permittivity to be isotropic? Note that it won't be at geometry interfaces due to subpixel, which I assume is not a problem here if the recorded data does not cover the waveguide interface? But in general why enforce this at all as opposed to compute absorption components from each of the field/permittivity components separately, and add them up?

@tomflexcompute
Copy link
Contributor

@alec-flexcompute can provide more comments?

@alec-flexcompute
Copy link
Contributor

Thanks for the feedback @momchil-flex ! Indeed, this example was developed partially as a way to test/familiarize myself with flexAgent. I agree it can just be removed, including the isotropic check. I'll make the corresponding edits shortly

@alec-flexcompute
Copy link
Contributor

Added the changes. I reran locally on my side as a sanity check and the results were the same

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.

4 participants