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

Fixed minor bugs and added functionality. #14

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AlexanderSchlager
Copy link

Thank you very much for the code, it has been a big help in my studies and research! I fixed some minor bugs, which might cause problems in very specific cases and added some functionality, if you want to include that.

(1) In structure_base.py on line 136, I changed a y_step to x_step (which has been mentioned in an issue as well), as it is inconsistent. The initial code causes errors if you use a non-uniform grid. There is also the fact that another stepsize is added to x, but not to y. I tried to fix this, but it caused further errors, so I assume this is on purpose.

(2) In structure_base.py on line 372, I removed a +1 stepsize for the discretized height. Even if you choose a stepsize for the layers, such that is correctly discretised (e.g. 10nm stepsize for a 100nm layer), this line of code will still add another self.y_step (in this case 10nm) to the layer thickness and cause it to be larger (110nm) instead. While this might not be an issue for small stepsizes or a low amount of layers, this causes problems for larger stepsizes or many layers, such as in Bragg-mirror structures.

(3) In mode_solver.py I added the functionality to save the x, y values in addition to the electric field amplitude. This was very useful for me in further simulations and I thought it might be useful for others. For now, it's very crude and simply overwrites the initially saved data, such that plotting still works the same, but it works.

If there is any reason as to why (1) and (2) have been done this way, please let me know! Thanks a lot!

Best regards
Alex

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.

1 participant