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

various bugs in adjoint solver #1405

Closed
oskooi opened this issue Oct 14, 2020 · 2 comments
Closed

various bugs in adjoint solver #1405

oskooi opened this issue Oct 14, 2020 · 2 comments
Labels

Comments

@oskooi
Copy link
Collaborator

oskooi commented Oct 14, 2020

The following four bugs were discovered using pylint.

  1. self.frequencies and self.monitor should be initialized to None.

class EigenmodeCoefficient(ObjectiveQuantitiy):
def __init__(self,sim,volume,mode,forward=True,kpoint_func=None,**kwargs):
'''
'''
self.sim = sim
self.volume=volume
self.mode=mode
self.forward = 0 if forward else 1
self.normal_direction = None
self.kpoint_func = kpoint_func
self.eval = None
self.EigenMode_kwargs = kwargs
return

same for:

class FourierFields(ObjectiveQuantitiy):
def __init__(self,sim,volume, component):
self.sim = sim
self.volume=volume
self.eval = None
self.component = component

and:

class Near2FarFields(ObjectiveQuantitiy):
def __init__(self,sim,Near2FarRegions, far_pt):
self.sim = sim
self.Near2FarRegions=Near2FarRegions
self.eval = None
self.far_pt = far_pt
return

  1. == should be replaced with =:

k0 == direction_scalar * mp.Vector3(z=1)

  1. There are two return statements rather than one:

return np.exp(-1j*2*np.pi*f0*t)
return np.where(n.any() < 0.0 or n.any() > self.N,0,np.exp(-1j*2*np.pi*f0*t))

  1. eta is not defined:

case1 = eta*npa.exp(-beta*(eta-x)/eta) - (eta-x)*npa.exp(-beta)
case2 = 1 - (1-eta)*npa.exp(-beta*(x-eta)/(1-eta)) - (eta-x)*npa.exp(-beta)

@oskooi oskooi added the bug label Oct 14, 2020
@smartalecH
Copy link
Collaborator

  1. I don't think python always requires initialization of class members to None... we use all of those variables throughout the code and haven't seen any issues.

  2. Yes this is a bug -- we haven't ever done tests with EM sources in the z-direction.

  3. The rect function here is legacy and not used. We use a nuttall window for the basis function.

  4. This along with several other filter bugs is fixed in WIP: Adjoint poynting flux #1398.

@oskooi
Copy link
Collaborator Author

oskooi commented Dec 30, 2021

Fixed by #1427, #1590, #1592, and #1593.

@oskooi oskooi closed this as completed Dec 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants