-
Notifications
You must be signed in to change notification settings - Fork 626
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
tutorial and unit test for mode sources using DiffractedPlanewave
object
#2107
Conversation
There is a test failure due to a segmentation fault related to garbage collection in
|
Codecov Report
@@ Coverage Diff @@
## master #2107 +/- ##
==========================================
+ Coverage 73.18% 73.26% +0.08%
==========================================
Files 17 17
Lines 4941 4941
==========================================
+ Hits 3616 3620 +4
+ Misses 1325 1321 -4
|
direction=mp.NO_DIRECTION, | ||
eig_kpoint=k, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's not working, make sure the MPB/Meep verbosity is > 1 and look at the NEW KPOINT
and MPB solved for frequency
verbose outputs — you should see that it is setting the parallel component of k
by the boundary conditions, and varying the perpendicular component of k
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there is a potential bug because printing out NEW KPOINT
shows that its value is clearly incorrect:
NEW KPOINT: 3, 2, 0
The y component (i.e., the periodic direction of the monitor monitor) should be 1.333... (i.e., 4/3) to match the k_point
of the Simulation
object. The reason it is not this value is due to these lines:
Lines 426 to 433 in 405b7e6
for (int i = 0; i < 3; ++i) { | |
k[i] = dot_product(R[i], kcart); // convert k to reciprocal basis | |
// G = inverse of R transpose, via cross-product formula | |
cross_product(G[i], R[(i + 1) % 3], R[(i + 2) % 3]); | |
double GdotR = dot_product(G[i], R[i]); | |
for (int j = 0; j < 3; ++j) | |
G[i][j] /= GdotR; | |
} |
In this example, R = (1,1.5,1)
and kcart = (0,1.33333,0)
which results in k = (0, 2.0, 0)
.
The fix seems to be to just replace k[i] = dot_product(R[i], kcart)
with k[i] = kcart[i]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, that code is correct — MPB expects the k point in the reciprocal-lattice basis, so it has to convert from the Cartesian basis of Meep. (It does the same thing with the DiffractedPlanewave
object.)
Instead, something weird is going on. You said the output is:
MPB solved for frequency_1(3,1.33333,0) = 2 after 29 iters
which means that it is taking the nonzero ky into account, but somehow it is getting the wrong kx — somehow, it is acting as though it is passing ky==0
to MPB, even though we are clearly printing that it is passing a nonzero k[1]
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I have finally gotten to the bottom of this issue. TLDR: the current behavior is a feature and not a bug.
Currently, to launch oblique planewaves using an EigenModeSource
with eig_band
set to an integer (rather than an DiffractedPlanewave
object), the user must provide its wavevector k
(a meep.Vector3
object) via direction=mp.NO_DIRECTION
and eig_kpoint=k
. This is because even though internally Meep sets the component of k
parallel to the line source (in the directions with periodic boundaries) to the components of the k_point
of the Simulation
object, the correct perpendicular component eig_band
is a DiffractedPlanewave
object.
It would be nice to extend this feature to when eig_band
is an integer but that can be a separate PR since it is not directly related to this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. A DiffractedPlanewave
object only enables the analytical calculation of k. Otherwise, it uses a Newton solver to find the k where ω(k) = specified frequency. This of course should be the same thing (but slower, and with more ambiguity about polarization and more difficulty in choosing a specific diffraction order).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The wavevector of the oblique planewave in Cartesian coordinates should be: k = (2.68742, 1.33333, 0.00000)
. However, the Newton solver shows:
MPB solved for frequency_1(3,1.33333,0) = 2 after 29 iters
update_maxwell_data_k:, k=(3.00000, 2.00000, 0.00000)
update_maxwell_data_k:, G[0]=(1.00000, 0.00000, 0.00000)
update_maxwell_data_k:, G[1]=(0.00000, 0.66667, 0.00000)
update_maxwell_data_k:, G[2]=(0.00000, 0.00000, 1.00000)
Note that the 1.33333
) is correct but the 3
) is not 2.68742
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turns out that there was a bug in the output of MPB's mode wavevector which is fixed by #2253. (The mode itself was correct.)
With the bug fix, the output from run_mode_source
of test_diffracted_planewave.py
in this PR is:
MPB solved for frequency_1(2.68742,1.33333,0) = 2 after 23 iters
This means k = (2.68742,1.33333,0)
which is equivalent to the k_point
of the Simulation
class object as expected.
Replaced by #2226. |
Closes #2072.
For some reason, I was not able to resolve the merge conflicts in
python/tests/test_diffracted_planewave.py
in #2072 during a rebase. This is why I just created this new PR.