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

bug in run-k-points but not run-k-point #770

Closed
acerjan opened this issue Mar 15, 2019 · 4 comments
Closed

bug in run-k-points but not run-k-point #770

acerjan opened this issue Mar 15, 2019 · 4 comments
Labels

Comments

@acerjan
Copy link
Contributor

acerjan commented Mar 15, 2019

There appears to be a slight but reproducible discrepancy between the frequencies generated between (run-k-points) and (run-k-point) for systems with non-zero k.

In the attached file, meep_debug.txt (it's clearly a scheme file but github isn't letting me upload it as .ctl), I'm simulating a photonic crystal slab embedded in a 3D photonic crystal environment comprised of an "extruded" 2D photonic crystal. (The structure is similar to the one shown in Fig. 2a of https://arxiv.org/abs/1901.07126.) The k-point currently specified is that of a bound state in the continuum.

When meep is run using (run-k-points), the frequency of the BIC is found to be 0.36130475184754957.

When meep is instead run using (run-k-point), the frequency of the BIC is found to be 0.35714322640725954.

Although these frequencies are very similar, they are not identical, as one would expect given that (run-k-points) is simply syntactic sugar for (map (lambda (k) (run-k-point)) kList) as is defined in line 1351 of https://github.com/NanoComp/meep/blob/master/scheme/meep.scm.in#L1351. More importantly, when using these frequencies in conjunction with other features available in meep, such as flux regions, this frequency difference is significant, as the Q factor is diverging. Even for non-BIC but high-Q choices of k, the difference in frequencies found between (run-k-points) and (run-k-point) is significant enough to effect results. These other calculations suggest that the correct frequency is found by (run-k-point).

I've done some debugging of this by copying in the definition of (run-k-points), and it appears that the offending line is line #1356 of meep.scm.in, or line 165 of the attached code, which has a call to (init-fields) prior to setting the k-point of the system. Commenting out this one line fixes the discrepancy. So, although I can see that the fields are reset after the k-point is changed in (run-k-point), it would appear that there is some lingering effect from the fields being initialized without the correct k-point.

If you would like, I can put together a quick pull request that deletes this line and moves the (output-epsilon) into the actual run command of (run-k-point), but this clearly isn't the ideal fix. I'd also like to get confirmation that we expect the results of (run-k-point) to be correct, and that you agree there is a slight bug with initializing the fields prior to setting the k-point.

Thanks!

meep_debug.txt

@stevengj
Copy link
Collaborator

stevengj commented Mar 21, 2019

A PR that removes that line is fine with me

Alternatively, to preserve the output-epsilon call (which isn't crucial), you can try replacing the (init-fields) call with a call to (change-k-point! k), which internally calls init-fields but sets up the boundary conditions properly. (I think the basic problem is that it was calling init-fields without setting the k-point.)

@acerjan
Copy link
Contributor Author

acerjan commented Mar 21, 2019

Ok. I'll put something together after I confirm that the problem isn't in the (reset-fields) portion of (run-k-point). i.e., the problem might be in starting the fields at one value of k, then resetting the fields with a different value of k. (this problem would show up in a single call of run-k-points, but would only show up in two calls of run-k-point.)

@acerjan
Copy link
Contributor Author

acerjan commented Mar 22, 2019

Ok, so either adding (change-k-point! k) or (set! k-point k) to the line prior to (init-fields) fixes the problem in my test code. Changing (init-fields) to (change-k-point! k) is throwing an error that fields are not initialized when an output-component is called.

However, there's still something I'm not understanding about how to actually fix the problem in meep's code. /meep/scheme/meep.scm seems to be regenerated when the code is recompiled, and even if I make changes to it without recompiling the results are unaffected. What is generating meep.scm and how to I fix this?

@oskooi
Copy link
Collaborator

oskooi commented Mar 22, 2019

Changes should be made to scheme/meep.scm.in rather than scheme/meep.scm since the latter is generated by SWIG as mainly a clone of the former during the build process. Any change to scheme/meep.scm will therefore be lost/ignored (as you have noticed).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants