Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Inconsistency in interface for get_eigenmode_coefficients and EigenModeSource constructor affecting adjoint solver #2755

Closed
oskooi opened this issue Jan 6, 2024 · 0 comments · Fixed by #2767

Comments

@oskooi
Copy link
Collaborator

oskooi commented Jan 6, 2024

There is an inconsistency in the interface for the related functions get_eigenmode_coefficients and the constructor for the EigenModeSource class object involving the argument eig_vol.

get_eigenmode_coefficients contains an argument eig_vol of type meep.Volume:

meep/python/simulation.py

Lines 4133 to 4143 in 0dc44da

def get_eigenmode_coefficients(
self,
flux: DftFlux,
bands: Union[List[int], DiffractedPlanewave],
eig_parity: int = mp.NO_PARITY,
eig_vol: Volume = None,
eig_resolution: float = 0,
eig_tolerance: float = 1e-12,
kpoint_func: Callable[[float, int], float] = None,
direction: int = mp.AUTOMATIC,
) -> NamedTuple:

The constructor for the EigenModeSource class object does not have an eig_vol argument and instead uses eig_lattice_size and eig_lattice_center:

meep/python/source.py

Lines 459 to 475 in 0dc44da

def __init__(
self,
src,
center=None,
volume=None,
eig_lattice_size=None,
eig_lattice_center=None,
component=mp.ALL_COMPONENTS,
direction=mp.AUTOMATIC,
eig_band=1,
eig_kpoint=Vector3(),
eig_match_freq=True,
eig_parity=mp.NO_PARITY,
eig_resolution=0,
eig_tolerance=1e-12,
**kwargs,
):

These two arguments are used to create an eig_vol object in a separate function add_source:

meep/python/source.py

Lines 646 to 651 in 0dc44da

eig_vol = mp.Volume(
self.eig_lattice_center,
self.eig_lattice_size,
sim.dimensions,
is_cylindrical=sim.is_cylindrical,
).swigobj

eig_vol and (eig_lattice_center, eig_lattice_size) are doing the same thing. It would be good to remove this inconsistency in the interface which currently prevents using eig_vol to define an meep.adjoint.EigenmodeCoefficient objective function because it is not possible to create an adjoint source. The problem is that the keyword arguments passed to EigenmodeCoefficient are passed directly to EigenModeSource in the function place_adjoint_source:

source = mp.EigenModeSource(
src,
eig_band=self.mode,
direction=mp.NO_DIRECTION,
eig_kpoint=eig_kpoint,
amplitude=amp,
eig_match_freq=True,
size=self.volume.size,
center=self.volume.center,
**self.eigenmode_kwargs,
)

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant