NanoComp / meep

free finite-difference time-domain (FDTD) software for electromagnetic simulations
GNU General Public License v2.0
1.22k stars 621 forks source link

inconsistent DFT fields monitors of the adjoint solver #1793

Open oskooi opened 3 years ago

oskooi commented 3 years ago

Currently, the EigenmodeCoefficient class of the adjoint solver defines the DFT field monitors on the Yee grid:

https://github.com/NanoComp/meep/blob/434f3fbf0ffe2334ce99cf105c6ec94ed44c95b7/python/adjoint/objective.py#L154-L162

However, the FourierFields class defines the DFT field monitors at the voxel centers:

https://github.com/NanoComp/meep/blob/434f3fbf0ffe2334ce99cf105c6ec94ed44c95b7/python/adjoint/objective.py#L239-L248

And yet, the Near2FarFields class defines the DFT field monitors on the Yee grid:

https://github.com/NanoComp/meep/blob/434f3fbf0ffe2334ce99cf105c6ec94ed44c95b7/python/adjoint/objective.py#L340-L347

Note: for Near2FarFields, the yee_grid option is not defined here but use_centered_grid (third last argument) is set to false in fields::add_near2far:

https://github.com/NanoComp/meep/blob/849bf3d3ad88aae0e38a48a9cd7454987db86c66/src/near2far.cpp#L635

https://github.com/NanoComp/meep/blob/434f3fbf0ffe2334ce99cf105c6ec94ed44c95b7/src/meep.hpp#L1961-L1966

When defining an objective function as part of an adjoint simulation, the user therefore needs to be consistent with these definitions. This is similar to the set up used in the unit test for the adjoint solver where the yee_grid option is explicitly defined:

https://github.com/NanoComp/meep/blob/434f3fbf0ffe2334ce99cf105c6ec94ed44c95b7/python/tests/test_adjoint_solver.py#L79-L91

Ensuring this type of consistency (particularly for the mode coefficients which does not use the default option) can be problematic and a source of error for the average user. It would be useful if the DFT fields monitors for all objective classes were always defined at the voxel centers.

ahoenselaar commented 3 years ago

Beyond consistency, are there reasons to favor monitors on the centered grid over monitors on the Yee grid?

The current implementation of monitors on the centered grid is inefficient because it performs interpolation whenever the monitor data is updated during time stepping. It would be more efficient to place DFT monitors on the Yee grid and conditionally perform interpolation when returning data to the user. This is supported by a linearity argument for the DFT and the interpolation step.

stevengj commented 3 years ago

The basic idea here is that the centered grid is much easier for the user to deal with than the Yee grid. And FourierFields are designed to be used directly by the user. e.g. it would be pretty hard for the user to compute ExH if all the field components are at different points in space.

However, it would be good to have the option to get FourierFields directly on the Yee grid, e.g. the version that @mawc2019 is working on.

Whereas the DFT fields for the eigenmode coefficients and the near2far transformations are mainly for use by Meep, and Meep knows how to handle Yee-grid data.

It would be more efficient to place DFT monitors on the Yee grid and conditionally perform interpolation when returning data to the user.

I agree: it would be mathematically equivalent to do the DFT accumulation on the Yee grid and only convert to the centered grid when we output to the user.

oskooi commented 2 years ago

Setting yee_grid=False for the EigenModeCoefficient class of objective.py is causing two of the gradient tests in test_adjoint_solver.py to fail in a significant way:

======================================================================
FAIL: test_adjoint_solver_eigenmode (__main__.TestAdjointSolver)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "./tests/test_adjoint_solver.py", line 320, in test_adjoint_solver_eigenmode
    self.assertClose(adjsol_obj,S12_unperturbed,epsilon=1e-6)
  File "/home/install/meep/python/tests/utils.py", line 28, in assertClose
    self.assertLessEqual(diff_norm, epsilon * np.maximum(x_norm, y_norm), msg)
AssertionError: 0.8343982855925702 not less than or equal to 3.0114380814839494e-05 : 

======================================================================
FAIL: test_gradient_backpropagation (__main__.TestAdjointSolver)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "./tests/test_adjoint_solver.py", line 362, in test_gradient_backpropagation
    self.assertClose(adjsol_obj,S12_unperturbed,epsilon=1e-6)
  File "/home/install/meep/python/tests/utils.py", line 28, in assertClose
    self.assertLessEqual(diff_norm, epsilon * np.maximum(x_norm, y_norm), msg)
AssertionError: 0.8266286252600565 not less than or equal to 2.9548554303440535e-05 : 

----------------------------------------------------------------------
Ran 4 tests in 131.852s

FAILED (failures=2)

@smartalecH, why was it necessary to override the yee_grid=False default (see below) of add_mode_monitor for EigenModeCoefficient? Is it just a matter of adding the interpolation step from the Yee to the centered grid to the backpropagation step when computing the gradients to enable using the centered grid for EigenModeCoefficient?

https://github.com/NanoComp/meep/blob/c65a1499c9e00d41d263d0034bbef03a6d093532/python/simulation.py#L3027

smartalecH commented 2 years ago

Yes that's expected... we must evaluate on the yee grid.

smartalecH commented 2 years ago

(...to continue using a flipped eigenmode source as our adjoint source. We can restrict the fields, in theory, if we store the field profiles at the monitors and process those accordingly, just like we do with the Near2Far.

Although we still need to finish #1547 before we can proceed with that approach.)