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

re-instate undocumented get_array_slice_dimensions in Python interface #1464

Merged
merged 1 commit into from Jan 8, 2021

Conversation

oskooi
Copy link
Collaborator

@oskooi oskooi commented Jan 6, 2021

#1456 removed get_array_slice_dimensions from the Python interface which unfortunately was discovered afterwards to be used by the adjoint solver in a couple of places:

# store design region voxel parameters
self.design_grids = []
for drm in self.design_region_monitors:
s = [self.sim.get_array_slice_dimensions(c,vol=drm.where)[0] for c in [mp.Ex,mp.Ey,mp.Ez]]
self.design_grids += [YeeDims(*s)]

# get monitor dft output array dimensions
dims = []
for m in mon:
ci_dims = []
for ci in c:
comp = ci if yee_grid else mp.Dielectric
ci_dims += [simob.get_array_slice_dimensions(comp,vol=m.where)[0]]
dims.append(ci_dims)

Rather than modify the above lines to use the low-level _get_array_slice_dimensions, this PR simply re-instates get_array_slice_dimensions (in its original form). Note that this is an internal function used only by the adjoint solver that is not documented in the user manual.

We will need to do a version 1.17.1 release after this is merged since 1.17.0 broke the adjoint solver.

Also, it would be good to add some unit tests for the adjoint solver to prevent these types of fixes in the future.

@oskooi oskooi requested a review from smartalecH January 6, 2021 21:38
@oskooi oskooi added the bug label Jan 7, 2021
Copy link
Collaborator

@smartalecH smartalecH left a comment

Choose a reason for hiding this comment

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

Looks good to me. I ran locally just to make sure we weren't missing anything else:

image

The bend tutorial gradients look fine.

I agree though -- a simple adjoint unit test would really help (it's on my list of things to do).

@smartalecH smartalecH mentioned this pull request Jan 7, 2021
@stevengj stevengj merged commit 21aee04 into NanoComp:master Jan 8, 2021
@oskooi oskooi deleted the get_array_slice_dim_reinstate branch January 8, 2021 19:33
bencbartlett pushed a commit to bencbartlett/meep that referenced this pull request Sep 9, 2021
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 this pull request may close these issues.

None yet

3 participants