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

incorrect PMLs for Cartesian coordinates in plot2D #1880

Closed
oskooi opened this issue Jan 4, 2022 · 1 comment · Fixed by #1884
Closed

incorrect PMLs for Cartesian coordinates in plot2D #1880

oskooi opened this issue Jan 4, 2022 · 1 comment · Fixed by #1884
Labels

Comments

@oskooi
Copy link
Collaborator

oskooi commented Jan 4, 2022

#1873 seems to have introduced a bug in plot2D for PMLs in Cartesian coordinates. The bug appears specifically for PMLs in the meep.X direction with side meep.Low as demonstrated in the following example. In the example, there should be a PML in the -x boundary of the cell. Separately, it would be good to add a unit test for plot2D to avoid these types of bugs in the future.

PR 1873 (missing PML in left boundary)
mie_scatt_plot2D_master

prior to PR 1873
mie_scatt_plot2D_pre1873

from meep.materials import Au
import meep as mp
import numpy as np
import matplotlib

n_H2O = 1.333 # refractive index of water                                                                                                                    

r = 1.0  # radius of sphere                                                                                                                                  

wvl_min = 2*np.pi*r/10
wvl_max = 2*np.pi*r/2

frq_min = 1/wvl_max
frq_max = 1/wvl_min
frq_cen = 0.5*(frq_min+frq_max)
dfrq = frq_max-frq_min
nfrq = 100

resolution = 100

dpml = 0.5*wvl_max
dair = 0.5*wvl_max

pml_layers = [mp.PML(thickness=dpml)]

s = 2*(dpml+dair+r)
cell_size = mp.Vector3(s,s,0)

sources = [mp.Source(mp.GaussianSource(frq_cen,fwidth=dfrq,is_integrated=True),
                     center=mp.Vector3(-0.5*s+dpml),
                     size=mp.Vector3(0,s,0),
                     component=mp.Ez)]

symmetries = [mp.Mirror(mp.Y)]

geometry = [mp.Sphere(material=Au,
                      center=mp.Vector3(),
                      radius=r)]

sim = mp.Simulation(resolution=resolution,
                    cell_size=cell_size,
                    boundary_layers=pml_layers,
                    sources=sources,
                    k_point=mp.Vector3(),
                    symmetries=symmetries,
                    default_material=mp.Medium(index=n_H2O),
                    geometry=geometry)

box_x1 = sim.add_flux(frq_cen, dfrq, nfrq, mp.FluxRegion(center=mp.Vector3(x=-r),size=mp.Vector3(0,2*r)))
box_x2 = sim.add_flux(frq_cen, dfrq, nfrq, mp.FluxRegion(center=mp.Vector3(x=+r),size=mp.Vector3(0,2*r)))
box_y1 = sim.add_flux(frq_cen, dfrq, nfrq, mp.FluxRegion(center=mp.Vector3(y=-r),size=mp.Vector3(2*r,0)))
box_y2 = sim.add_flux(frq_cen, dfrq, nfrq, mp.FluxRegion(center=mp.Vector3(y=+r),size=mp.Vector3(2*r,0)))

sim.plot2D()
plt.savefig('bug_plot2D.png',dpi=150,bbox_inches='tight')

cc @smartalecH

@oskooi oskooi added the bug label Jan 4, 2022
@smartalecH
Copy link
Collaborator

#1873 seems to have introduced a bug in plot2D for PMLs in Cartesian coordinates.

Whoops. Easy fix.

The lines here:

if (permutation[0] == mp.X) and (permutation[1] == mp.Low):
continue

here:
if (boundary.direction == mp.X) and (side == mp.Low):
continue

and here:
if (boundary.direction == mp.X) and (boundary.side == mp.Low):
continue

just need to check for cylindrical coordinates (like here):
elif sim.dimensions == mp.CYLINDRICAL or sim.is_cylindrical:

Separately, it would be good to add a unit test for plot2D to avoid these types of bugs in the future.

We already have a unit test (see here). There's just no easy way to test if an image is "right".

We currently hash the image and make sure it's reasonably similar to what's new.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants