[1/13][Adjoint Module] Fix MaterialGrid design-variable attribution in the adjoint gradient - #3297
Open
smartalecH wants to merge 5 commits into
Open
smartalecH wants to merge 5 commits into
smartalecH wants to merge 5 commits into
Conversation
A design region whose MaterialGrid touches another MaterialGrid gets a wrong
adjoint gradient, and, when the neighbour's grid is larger, an out-of-bounds
write.
material_grids_addgradient_point() resolves the material grid by a spatial
lookup at the point (geom_tree_search) and then interpolates into the supplied
v[] using *that* grid's grid_size and box coordinates. It never checks that the
grid it found is the one the design region being processed actually owns; the
comment above the loop says as much, and leaves it to the user to "only have one
unique design grid in this volume".
That is not something the user can arrange. add_dft_fields() snaps the design
region's monitor outward to enclosing grid nodes, so two abutting grids give one
region a monitor that covers a row of nodes lying inside the other grid's
object. Instrumented on a 2d case with two 21x11-node regions sharing a
boundary, the lower region's gradient pass touched 210 nodes of its own grid and
21 of the neighbour's -- exactly the shared row. Their contributions are
interpolated in the neighbour's box coordinates and summed into this region's
v[]: wrong values when the two grids are the same size, and a write past the end
of v[] when the neighbour's grid is larger (reproducibly aborts in malloc).
Fix by giving every Python MaterialGrid a stable id, carrying it into
material_data, and passing the design region's id down so a point that resolves
to a different grid is skipped. The id is per Python object, not per
material_data, so the documented case of one MaterialGrid backing several
geometric objects (symmetries) still accumulates from all of them. Nothing is
lost by skipping: the neighbouring design region visits those same nodes on its
own pass. owner_grid_id < 0 disables the filter.
Measured, adjoint / directional finite difference:
before after
2d, one design region 1.0000 1.0000
2d, two regions, 1 px apart 1.0000 1.0000
2d, two regions abutting 1.3508 1.0001
2d, abutting, interface off-node 0.3360 1.0000
3d, two 220 nm sublayers, lower 0.33-1.31 1.0000-1.0002
3d, two 220 nm sublayers, upper 2.35-5.46 1.0000
Not a chunk effect: num_chunks is 1 in the 2d cases above. Not subpixel
smoothing either -- eps_averaging=False makes it worse, since the mixing is in
the grid lookup rather than in the averaging. A design region abutting a static
(non-MaterialGrid) block is unaffected, which is what isolates this to two
adjacent grids.
The regression test covers the adjacent case twice: with equal grid sizes, where
a regression is a wrong value and fails as an assertion, and with unequal sizes,
where it is also the out-of-bounds write. Both run in serial in ~10 s.
test_adjoint_solver 13/13, test_adjoint_chunks + test_adjoint_utils +
test_subpixel_3d 12/12.
Note: material_grids_addgradient() gains a required owner_grid_id parameter
before du. The only in-tree caller is _get_gradient in python/meep.i.
Fixes #1984. A design constrained to a symmetry by overlapping one MaterialGrid with transformed copies of itself got an adjoint gradient that was not the gradient -- wrong sign, not merely wrong scale. material_grids_addgradient_point() walked the overlapping copies and differentiated each in turn, but add_interpolate_weights() was handed the *first* copy's weights array, hoisted before the loop, while the stencil index came from the current copy's box coordinates. For a transformed copy those disagree: the finite difference in get_material_gradient() perturbs a sample that takes no part in that copy's interpolation. Untransformed overlaps survived by cancellation -- the indices coincide, so the second copy just repeats the first, and `scalegrad /= matgrid_val_count` divides the duplicate back out. That divide was itself a double count: get_material_gradient() finite-differences through matgrid_val(), so the 1/N of a U_MEAN average is already in the result. Restructured so the unit of differentiation is the design variable rather than the copy. Collect the copies of the design region's own grid that cover the point, take the union of their interpolation stencils, and for each variable in it perturb that entry in *every* copy at once. Copies are recognised by the grid_id added in the parent commit, which is what makes "the same design variable" expressible at this level. `scalegrad /= matgrid_val_count` is gone. This subsumes the parent commit's U_DEFAULT-scoped guard: filtering at collection time is correct for every grid_type, so the scoping is dropped. Measured, adjoint / directional finite difference: before after overlapped with a 90-degree rotation -0.1598 1.0003 overlapped with a mirror 0.2433 1.0000 overlapped, untransformed 1.0000 1.0000 overlapped, distinct grids 1.0000 1.0000 two adjacent design regions 1.3508 0.9986 single design region 1.0000 1.0000 U_MIN and U_PROD keep their existing value-dependent adjustments, applied to the single copy the caller allows for those kinds; overlapping grids of those kinds are still rejected upstream. Not fixed here, and pre-existing: an overlap whose second grid is left at all zeros reads 1.0077. It is step-independent, so not finite-difference error, and it moves neither with this change nor with the parent commit. test_adjoint_solver, test_adjoint_chunks, test_adjoint_utils, test_adjoint_cyl, test_adjoint_jax, test_adjoint_adjacent_grids 37/37; test_material_grid and test_subpixel_3d 8/8.
smartalecH
changed the title
stevengj
reviewed
Sep 4, 2026
Comment on lines
+2760
to
+2762
| /* U_MIN and U_PROD carry adjustments defined in terms of the interpolated | ||
| value at this point. Overlapping grids of these kinds are rejected by | ||
| the caller, so there is exactly one copy and this is unchanged. */ |
Collaborator
There was a problem hiding this comment.
What does it mean that "overlapping grids of these kinds are rejected by the caller"? The whole point of U_MIN and U_PROD is that they apply to overlapping grids.
Collaborator
|
The overall approach seems fine (adding an id per material grid and using that when differentiating with respect to the variables in a single grid), but I want to look closely at some of the details. One comment in the code, noted above, was disturbing. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #3277.
Fixes #1984.
Stacked on #3274 — based on
fix/adjoint-chunk-pairing-minimal, so the diff here is just these two commits. Merge #3274 first, then this retargets tomastercleanly.While working on a new 3D grating coupler TO example for meep (2 design regions) I noticed that the gradients were rather inaccurate. Claude helped me trace things down to the unique case where we have two adjacent design regions (e.g. a partial etched layer). There were different issues depending on the sizes of the design regions (e.g. if one design region was bigger than the other, then we got a segfault!)
The core issue was that the logic we had for handling multiple design grids during the recombination step wasn't properly bookkeeping the effects near the boundaries. This was because we weren't keeping track of "which grid was which." In addition, we had some hairy "copies" of grids we were trying to use to keep track of multiple grids (hence the crash). So the resulting gradient near the boundary was completely off. To fix this, we simply add a grid id (automatically populated in python) and propagated to the cpp backend via swig.
Practically speaking, this means that the unit of differentiation is now the design variable, not the grid copy.
Turns out this bug also affects symmetries (see #1984) and the fix also resolves it.