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

flush subnormals on x86 #1709

Merged
merged 6 commits into from
Jul 29, 2021
Merged

flush subnormals on x86 #1709

merged 6 commits into from
Jul 29, 2021

Conversation

stevengj
Copy link
Collaborator

Closes #1708.

@ahoenselaar
Copy link
Contributor

I wonder whether this could negatively affect the source cutoff code here:
https://github.com/NanoComp/meep/blob/master/src/sources.cpp#L70

@stevengj
Copy link
Collaborator Author

I'm not sure how it would affect that cutoff calculation, could you elaborate? (1e-100 is not subnormal in double precision, which is used for that calculation.)

@ahoenselaar
Copy link
Contributor

Oh right, the computation is always done in double precision. Should be fine then.

@stevengj
Copy link
Collaborator Author

stevengj commented Jul 28, 2021 •
edited
Loading

That being said, this 1e-100 cutoff (and any similar cutoffs elsewhere in the code) should really be increased when the fields are single precision. (However, this particular constructor is not used in the Python API if I recall correctly.)

@codecov-commenter
Copy link

codecov-commenter commented Jul 28, 2021 •
edited
Loading

Codecov Report

Merging #1709 (176db3b) into master (b07a6ce) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1709   +/-   ##
=======================================
  Coverage   73.33%   73.34%           
=======================================
  Files          13       13           
  Lines        4527     4528    +1     
=======================================
+ Hits         3320     3321    +1     
  Misses       1207     1207
Impacted Files Coverage Δ
python/simulation.py 76.25% <100.00%> (+0.01%) ⬆️

@oskooi
Copy link
Collaborator

oskooi commented Jul 28, 2021

This seems to have fixed the issue with the noticeable slowdown in the timestepping due to the turn on of the pulsed source for a test problem in 3d:

PR1709

I'm not sure why there is a failing C++ unit test (harmonics.cpp) for single-precision fields due to a small mismatch in the relative tolerance. Perhaps we can change that in a separate PR?

FAIL: harmonics
===============

Using MPI version 3.1, 2 processes
harmonics(2.7e-05,0.0001,1) = 9.80387e-07, 9.99195e-07
error: 3rd harmonic mismatches known val
 --- 9.99195e-07 vs. 9.99349e-07 (0.000154525 error > 0.0001)

@oskooi
Copy link
Collaborator

oskooi commented Jul 28, 2021

I tested this branch on my local machine and the same failing test on CI harmonics.cpp also failed. The same test (along with all the other unit tests in the make check suite) pass using the master branch. This means something about this branch is causing a small change in the Harminv results in harmonics.cpp when using single precision.

@stevengj
Copy link
Collaborator Author

I think the tolerance in harmonics.cpp (and in some of the other tests) was simply overly aggressive for single precision, so it was vulnerable to failure from slight changes in roundoff behavior.

@stevengj stevengj merged commit ef18156 into master Jul 29, 2021
@stevengj stevengj deleted the subnormals branch July 29, 2021 00:08
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
@stevengj
flush subnormals on x86 (NanoComp#1709)
8edcb6b 
* 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
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
@stevengj @mawc2019
flush subnormals on x86 (NanoComp#1709)
9c551d8 
* 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
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>
@smartalecH smartalecH mentioned this pull request Sep 15, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

flush zero subnormals
4 participants
@stevengj @ahoenselaar @codecov-commenter @oskooi