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

Enable nonzero k_point in adjoint solver #1705

Merged
merged 26 commits into from
Jul 30, 2021
Merged

Conversation

mochen4
Copy link
Collaborator

@mochen4 mochen4 commented Jul 27, 2021

Fixes #1667. The test_adjoint_solver.py is updated correspondingly to include when k_point is False, zero, and nonzero.

@mochen4
Copy link
Collaborator Author

mochen4 commented Jul 27, 2021

The original setup of the EigenmodeCoefficient test in the test_adjoint_solver.py has the source and monitor coincide with the sides of the PMLs, and extends into the PMLs of the other direction. The adjoint gradients and directional derivates won't match if k_point is nonzero; if we use the original calculate_fd_gradient() in optimization_problem.py that calculates the fd_gradients in unit directions, the fields become nan. (The forward and adjoint run are both okay, but the fields are nan after single pixel is perturbed during calculate_fd_gradient().)

Using the setup of the tutorial example, the gradients agree with nonzero k_point.

If the source and monitor don't extend into the PMLs, then the adjoint gradients will match the directional derivates, but the fields are still nan when using calculate_fd_gradient(). If we then moved the source and monitor a little bit away from the PMLs, everything will work fine; so I further modified the test this way.

@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2021

Codecov Report

Merging #1705 (fb0b3b2) into master (83a10c5) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1705      +/-   ##
==========================================
+ Coverage   73.32%   73.35%   +0.02%     
==========================================
  Files          13       13              
  Lines        4525     4530       +5     
==========================================
+ Hits         3318     3323       +5     
  Misses       1207     1207              
Impacted Files Coverage Δ
python/adjoint/objective.py 75.30% <100.00%> (+0.30%) ⬆️
python/adjoint/optimization_problem.py 69.58% <100.00%> (+0.23%) ⬆️
python/simulation.py 76.25% <100.00%> (+0.01%) ⬆️

@mochen4
Copy link
Collaborator Author

mochen4 commented Jul 27, 2021

In the context of the PR #1704, the factor of 2 is needed whenever force_complex_fields=False. However, when k_point is nonzero, we may not need the factor even if we don't force complex fields.

I wonder if there are other cases where we don't need the factor even if force_complex_fields=False; and what will be the best way to check whether indeed only real fields are used.

@oskooi
Copy link
Collaborator

oskooi commented Jul 27, 2021

Looks like the failing test is test_adjoint_solver.py for the case in which Meep is compiled with single precision (#1698). Perhaps you need to just lower the tolerance some place when mp.is_single_precision() is true?

ahoenselaar and others added 3 commits July 27, 2021 15:23
* Fix the `binary_partition` copy constructor.

Prevents issues with older compilers.
\NanoComp#1683 NanoComp#1701

* Update src/structure_dump.cpp

Co-authored-by: Steven G. Johnson <[email protected]>

Co-authored-by: Steven G. Johnson <[email protected]>
@mochen4
Copy link
Collaborator Author

mochen4 commented Jul 28, 2021

Thanks Ardavan! I lowered the tolerance for single precision, and it works!

…anoComp#1704)

* fix factor 2

* fix

* fix

* fix

* single precision test tol

* single precision test tol

* Update python/adjoint/objective.py

Co-authored-by: Steven G. Johnson <[email protected]>

Co-authored-by: Mo Chen <[email protected]>
Co-authored-by: Steven G. Johnson <[email protected]>
stevengj and others added 14 commits July 28, 2021 16:32
* assertVectorsClose works for scalars as well as vectors

* rename ApproxComparisonMixin -> ApproxComparisonTestCase, since it is a subclass of TestCase, and use it as such
…mp#1713)

* add --with-coverage to control usage of Python coverage tests

* move  --with-coverage  to correct CI line
* flush subnormals on x86

* less aggressive error threshold in single precision

* increase single-precision tolerance

* update tols in test_array_metadata

* update for assertVectorsClose rename

* lower tolerance in single precision
…ision (NanoComp#1714)

* lower tolerance of CW and eigenfrequency solver tests for single precision

* remove change in default argument from Python wrappers
Mo Chen added 2 commits July 29, 2021 23:50
@stevengj stevengj merged commit 5011d4d into NanoComp:master Jul 30, 2021
bencbartlett pushed a commit to bencbartlett/meep that referenced this pull request Sep 9, 2021
* fix factor 2

* fix

* fix

* fix

* kpoint

* fix

* Fix the `binary_partition` copy constructor. (NanoComp#1702)

* Fix the `binary_partition` copy constructor.

Prevents issues with older compilers.
\NanoComp#1683 NanoComp#1701

* Update src/structure_dump.cpp

Co-authored-by: Steven G. Johnson <[email protected]>

Co-authored-by: Steven G. Johnson <[email protected]>

* single precision test tol

* single precision test tol

* Fix factor of 2 in adjoint gradients when not using complex fields (NanoComp#1704)

* fix factor 2

* fix

* fix

* fix

* single precision test tol

* single precision test tol

* Update python/adjoint/objective.py

Co-authored-by: Steven G. Johnson <[email protected]>

Co-authored-by: Mo Chen <[email protected]>
Co-authored-by: Steven G. Johnson <[email protected]>

* assertVectorsClose works for scalars as well as vectors (NanoComp#1712)

* assertVectorsClose works for scalars as well as vectors

* rename ApproxComparisonMixin -> ApproxComparisonTestCase, since it is a subclass of TestCase, and use it as such

* add --with-coverage to control usage of Python coverage tests (NanoComp#1713)

* add --with-coverage to control usage of Python coverage tests

* move  --with-coverage  to correct CI line

* flush subnormals on x86 (NanoComp#1709)

* flush subnormals on x86

* less aggressive error threshold in single precision

* increase single-precision tolerance

* update tols in test_array_metadata

* update for assertVectorsClose rename

* lower tolerance in single precision

* Lower tolerance of CW and eigenfrequency solver tests for single precision (NanoComp#1714)

* lower tolerance of CW and eigenfrequency solver tests for single precision

* remove change in default argument from Python wrappers

* fix factor 2

* fix

* kpoint

* fix

* single precision test tol

* single precision test tol

* using real

* using real

* fix rebase

* fix

* fix

Co-authored-by: Mo Chen <[email protected]>
Co-authored-by: Andreas Hoenselaar <[email protected]>
Co-authored-by: Steven G. Johnson <[email protected]>
Co-authored-by: Ardavan Oskooi <[email protected]>
mawc2019 pushed a commit to mawc2019/meep that referenced this pull request Nov 3, 2021
* fix factor 2

* fix

* fix

* fix

* kpoint

* fix

* Fix the `binary_partition` copy constructor. (NanoComp#1702)

* Fix the `binary_partition` copy constructor.

Prevents issues with older compilers.
\NanoComp#1683 NanoComp#1701

* Update src/structure_dump.cpp

Co-authored-by: Steven G. Johnson <[email protected]>

Co-authored-by: Steven G. Johnson <[email protected]>

* single precision test tol

* single precision test tol

* Fix factor of 2 in adjoint gradients when not using complex fields (NanoComp#1704)

* fix factor 2

* fix

* fix

* fix

* single precision test tol

* single precision test tol

* Update python/adjoint/objective.py

Co-authored-by: Steven G. Johnson <[email protected]>

Co-authored-by: Mo Chen <[email protected]>
Co-authored-by: Steven G. Johnson <[email protected]>

* assertVectorsClose works for scalars as well as vectors (NanoComp#1712)

* assertVectorsClose works for scalars as well as vectors

* rename ApproxComparisonMixin -> ApproxComparisonTestCase, since it is a subclass of TestCase, and use it as such

* add --with-coverage to control usage of Python coverage tests (NanoComp#1713)

* add --with-coverage to control usage of Python coverage tests

* move  --with-coverage  to correct CI line

* flush subnormals on x86 (NanoComp#1709)

* flush subnormals on x86

* less aggressive error threshold in single precision

* increase single-precision tolerance

* update tols in test_array_metadata

* update for assertVectorsClose rename

* lower tolerance in single precision

* Lower tolerance of CW and eigenfrequency solver tests for single precision (NanoComp#1714)

* lower tolerance of CW and eigenfrequency solver tests for single precision

* remove change in default argument from Python wrappers

* fix factor 2

* fix

* kpoint

* fix

* single precision test tol

* single precision test tol

* using real

* using real

* fix rebase

* fix

* fix

Co-authored-by: Mo Chen <[email protected]>
Co-authored-by: Andreas Hoenselaar <[email protected]>
Co-authored-by: Steven G. Johnson <[email protected]>
Co-authored-by: Ardavan Oskooi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adjoint solver with oblique planewave sources
5 participants