NanoComp / meep

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

Increase array size of material grid by 1 in each dimension #2504

Closed mawc2019 closed 1 year ago

mawc2019 commented 1 year ago

The missing +1 in the array size of the material grid is restored in this PR, but some tests in test_adjoint_solver.py fail.

review-notebook-app[bot] commented 1 year ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

codecov-commenter commented 1 year ago

Codecov Report

Merging #2504 (0d73d36) into master (79e8d17) will not change coverage. The diff coverage is 100.00%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##           master    #2504   +/-   ##
=======================================
  Coverage   73.92%   73.92%           
=======================================
  Files          18       18           
  Lines        5296     5296           
=======================================
  Hits         3915     3915           
  Misses       1381     1381           
Impacted Files Coverage Δ
python/adjoint/filters.py 79.89% <100.00%> (ø)
mawc2019 commented 1 year ago

We need to increase some tolerances in test_adjoint_solver.py.

Python 3.7:

FAIL: test_eigenmode (__main__.TestAdjointSolver)
Verifies that the adjoint gradient for an objective function based
----------------------------------------------------------------------
Traceback (most recent call last):
  File "../../../python/tests/test_adjoint_solver.py", line 592, in test_eigenmode
    self.assertClose(adj_dd, fnd_dd, epsilon=tol)
  File "/home/runner/work/meep/meep/build/meep-1.27.0-beta/python/tests/utils.py", line 30, in assertClose
    self.assertLessEqual(diff_norm, epsilon * np.maximum(x_norm, y_norm), msg)
AssertionError: 2.4692787936065122e-05 not less than or equal to 2.147141650190434e-05 : 

======================================================================
FAIL: test_ldos (__main__.TestAdjointSolver)
Verifies that the adjoint gradient for an objective function based
----------------------------------------------------------------------
Traceback (most recent call last):
  File "../../../python/tests/test_adjoint_solver.py", line 621, in test_ldos
    self.assertClose(adj_dd, fnd_dd, epsilon=tol)
  File "/home/runner/work/meep/meep/build/meep-1.27.0-beta/python/tests/utils.py", line 30, in assertClose
    self.assertLessEqual(diff_norm, epsilon * np.maximum(x_norm, y_norm), msg)
AssertionError: 1.866014771444676e-10 not less than or equal to 7.504694841953264e-11 : 

Python 3.10:

FAIL: test_DFT_fields (__main__.TestAdjointSolver)
Verifies that the adjoint gradient for an objective function based
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/meep/meep/build/meep-1.27.0-beta/_build/sub/python/../../../python/tests/test_adjoint_solver.py", line 551, in test_DFT_fields
    self.assertClose(adj_dd, fnd_dd, epsilon=tol)
  File "/home/runner/work/meep/meep/build/meep-1.27.0-beta/python/tests/utils.py", line 30, in assertClose
    self.assertLessEqual(diff_norm, epsilon * np.maximum(x_norm, y_norm), msg)
AssertionError: 0.00032576555598241895 not less than or equal to 0.00015930181276331502 : 

======================================================================
FAIL: test_eigenmode (__main__.TestAdjointSolver)
Verifies that the adjoint gradient for an objective function based
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/meep/meep/build/meep-1.27.0-beta/_build/sub/python/../../../python/tests/test_adjoint_solver.py", line 592, in test_eigenmode
    self.assertClose(adj_dd, fnd_dd, epsilon=tol)
  File "/home/runner/work/meep/meep/build/meep-1.27.0-beta/python/tests/utils.py", line 30, in assertClose
    self.assertLessEqual(diff_norm, epsilon * np.maximum(x_norm, y_norm), msg)
AssertionError: 6.287598863467803e-05 not less than or equal to 3.214987858228824e-05 : 

======================================================================
FAIL: test_ldos (__main__.TestAdjointSolver)
Verifies that the adjoint gradient for an objective function based
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/meep/meep/build/meep-1.27.0-beta/_build/sub/python/../../../python/tests/test_adjoint_solver.py", line 621, in test_ldos
    self.assertClose(adj_dd, fnd_dd, epsilon=tol)
  File "/home/runner/work/meep/meep/build/meep-1.27.0-beta/python/tests/utils.py", line 30, in assertClose
    self.assertLessEqual(diff_norm, epsilon * np.maximum(x_norm, y_norm), msg)
AssertionError: 2.0344705001442236e-10 not less than or equal to 1.5012644966061473e-10 : 
mawc2019 commented 1 year ago

The test in test_unfilter_design also fails due to mismatch of array size.

ValueError: cannot reshape array of size 160000 into shape (401,401)
stevengj commented 1 year ago

A factor of ≈2 increase in the tolerances seems fine.

oskooi commented 1 year ago

Looks like the error tolerances in test_adjoint_solver.py need to be reverted to (close to) what they were in #1769.

mawc2019 commented 1 year ago

Looks like the error tolerances in test_adjoint_solver.py need to be reverted to (close to) what they were in https://github.com/NanoComp/meep/pull/1769.

We probably cannot do so because frequency widths are quite different in the two cases. Now there is df = 0.05*fcen, but in #1769 there was df = 0.23*fcen. The values of gradients and hence the tolerances are not comparable between the two cases.

In addition, it seems that +1 is also needed in the expressions of Nr and Nz in test_adjoint_cyl.py.

Nr, Nz = int(design_r * design_region_resolution), int(design_z * design_region_resolution)