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.

Sign up for GitHub

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

Jump to bottom

Fix factor of 2 in adjoint gradients when not using complex fields #1704

Merged
merged 7 commits into from
Jul 28, 2021
Merged

Fix factor of 2 in adjoint gradients when not using complex fields #1704

merged 7 commits into from
Jul 28, 2021

Conversation

mochen4
Copy link
Collaborator

@mochen4 mochen4 commented Jul 27, 2021 •
edited
Loading

Fixes #1685. The test_adjoint_solver.py is updated correspondingly to include when force_complex_fields=True.

@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2021 •
edited
Loading

Codecov Report

Merging #1704 (3a026bd) into master (83a10c5) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1704      +/-   ##
==========================================
+ Coverage   73.32%   73.33%   +0.01%     
==========================================
  Files          13       13              
  Lines        4525     4527       +2     
==========================================
+ Hits         3318     3320       +2     
  Misses       1207     1207
Impacted Files Coverage Δ
python/adjoint/objective.py 75.30% <100.00%> (+0.30%) ⬆️

@mochen4 mochen4 mentioned this pull request Jul 27, 2021
Merged
@mochen4
Copy link
Collaborator Author

mochen4 commented Jul 27, 2021 •
edited
Loading

I am very confused why the "CI/Test Python 3.9 with MPI (true)" failed while the other three passed. In fact, the test that failed is test_adjoint_solver.py. Moreover, running it (python3.9 and MPI) locally actually works.

@mochen4 @stevengj
Update python/adjoint/objective.py
8c5d1e8 
Co-authored-by: Steven G. Johnson <stevenj@mit.edu>
@stevengj stevengj merged commit ef2c9ca into NanoComp:master Jul 28, 2021
stevengj added a commit that referenced this pull request Jul 30, 2021
@mochen4 @ahoenselaar
@stevengj @oskooi
Enable nonzero k_point in adjoint solver (#1705)
5011d4d 
* fix factor 2

* fix

* fix

* fix

* kpoint

* fix

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

* Fix the `binary_partition` copy constructor.

Prevents issues with older compilers.
\#1683 #1701

* Update src/structure_dump.cpp

Co-authored-by: Steven G. Johnson <stevenj@mit.edu>

Co-authored-by: Steven G. Johnson <stevenj@mit.edu>

* single precision test tol

* single precision test tol

* Fix factor of 2 in adjoint gradients when not using complex fields (#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 <stevenj@mit.edu>

Co-authored-by: Mo Chen <mochen@Mos-MacBook-Pro.local>
Co-authored-by: Steven G. Johnson <stevenj@mit.edu>

* assertVectorsClose works for scalars as well as vectors (#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 (#1713)

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

* move  --with-coverage  to correct CI line

* flush subnormals on x86 (#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 (#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 <mochen@Mos-MacBook-Pro.local>
Co-authored-by: Andreas Hoenselaar <ahoens@google.com>
Co-authored-by: Steven G. Johnson <stevenj@mit.edu>
Co-authored-by: Ardavan Oskooi <ardavano@google.com>
bencbartlett pushed a commit to bencbartlett/meep that referenced this pull request Sep 9, 2021
@mochen4 @stevengj
Fix factor of 2 in adjoint gradients when not using complex fields (N…
f35b0f5 
…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 <stevenj@mit.edu>

Co-authored-by: Mo Chen <mochen@Mos-MacBook-Pro.local>
Co-authored-by: Steven G. Johnson <stevenj@mit.edu>
bencbartlett pushed a commit to bencbartlett/meep that referenced this pull request Sep 9, 2021
@mochen4 @ahoenselaar
@stevengj @oskooi
Enable nonzero k_point in adjoint solver (NanoComp#1705)
6feba73 
* 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 <stevenj@mit.edu>

Co-authored-by: Steven G. Johnson <stevenj@mit.edu>

* 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 <stevenj@mit.edu>

Co-authored-by: Mo Chen <mochen@Mos-MacBook-Pro.local>
Co-authored-by: Steven G. Johnson <stevenj@mit.edu>

* 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 <mochen@Mos-MacBook-Pro.local>
Co-authored-by: Andreas Hoenselaar <ahoens@google.com>
Co-authored-by: Steven G. Johnson <stevenj@mit.edu>
Co-authored-by: Ardavan Oskooi <ardavano@google.com>
mawc2019 pushed a commit to mawc2019/meep that referenced this pull request Nov 3, 2021
@mochen4 @stevengj @mawc2019
Fix factor of 2 in adjoint gradients when not using complex fields (N…
4c87c25 
…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 <stevenj@mit.edu>

Co-authored-by: Mo Chen <mochen@Mos-MacBook-Pro.local>
Co-authored-by: Steven G. Johnson <stevenj@mit.edu>
mawc2019 pushed a commit to mawc2019/meep that referenced this pull request Nov 3, 2021
@mochen4 @ahoenselaar
@stevengj @oskooi @mawc2019
Enable nonzero k_point in adjoint solver (NanoComp#1705)
3033f38 
* 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 <stevenj@mit.edu>

Co-authored-by: Steven G. Johnson <stevenj@mit.edu>

* 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 <stevenj@mit.edu>

Co-authored-by: Mo Chen <mochen@Mos-MacBook-Pro.local>
Co-authored-by: Steven G. Johnson <stevenj@mit.edu>

* 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 <mochen@Mos-MacBook-Pro.local>
Co-authored-by: Andreas Hoenselaar <ahoens@google.com>
Co-authored-by: Steven G. Johnson <stevenj@mit.edu>
Co-authored-by: Ardavan Oskooi <ardavano@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stevengj stevengj stevengj left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Missing factor of 2 in adjoint gradients for force_complex_fields=True
3 participants
@mochen4 @codecov-commenter @stevengj