legume
legume copied to clipboard
Rebased fields module staging
Now only 2 files have been changed.
The fields module which I feel should be a seperate from viz, as it contains a class.
Utils which has been merged with legume utils.
Some methods in utils are not related to the fields module but I don't see a reason to PR them seperately.
Both have had YAPF applied to them.
Overall this looks great now (I agree with minor comments from Ian). To make it actually useful for people, we'd need to document the relevant Fields methods, and also add an example. Could you prepare a notebook briefly showing the various functionalities? Or amend some of the existing notebooks, if you think that makes more sense?
Thanks Ian for the comments on code style. They are very much appreciated I will take a look.
Regarding examples, I have two notebooks that may suffice, but was unsure how to integrate into the current docs. Would you suggest just adding them as examples 09 and 10? There a numbers of .rst and other files in the docs folder. I am not familiar with their function. Is there anything that needs to be done to these files to properly add an example?
I can handle the proper documentation integration if you're not familiar with how sphinx works. Maybe just add a single notebook example and I'll take it from there. We don't need two separate notebooks if they cover more or less the same thing, but if they showcase different things then sure add them both. Thanks!
Two tutorials have been added. Both demonstrating different features of the fields module. Feedback is welcome.
Some of the features displayed in the waveguide tutorial were originally meant for photonic crystal ring resonators. However I built those structures using tech not in this module. At some point in the future it may be worth updating the tutorial with PCRRs instead of a waveguide but I don't think it is critical.
Sorry this took a while, I finally had a look at the examples. I think the functionality is pretty nice. I also think it can be wrapped up in a single example since the waveguide notebook seems to capture almost everything, I don't see the need for a separate cavity example (but I'll do that myself). However, when working through the waveguide example I gathered a few further comments/questions.
- I think polarization should not be a required argument but set to
None(not'None', see also Ian's comment above). - I cannot imagine a scenario in which the user would want to modulate with a k-vector different than the mode k-vector. Basically the question should be "do you want to see the periodic part only or the full Bloch field"? In the
viz.fieldfunction, this is handled by aperiodic=Truekwarg. My suggestion is to replace themodulationargument in theXYFieldconstructor with the same boolean kwarg, and use the k of the mode as modulation if set toFalse. Am I missing something? - Similarly, I don't understand the part of the example where you showcase the mode multiplied by an extra modulation,
Is there a use case for something like this? I think overriding the operators is nice, I just find this example confusing.modulation = np.exp(-1j*field_1.meshgrid[0]*2*np.pi) field_modulated = field_1 * modulation
Finally, I generally agree with all of Ian's comments too so it would be great if you could address these final points before we merge. I think the docs will need a bit of polishing but I can do that eventually.
Hi Momchil,
No worries about the delay. I have been swamped this past month so haven't got to Ian's suggestions either. I will have time to return to this this week.
WRT to the modulation thank you very much for pointing that out. It was the result of a misunderstanding of Bloch's theorem on my part that I have now corrected. I completely agree it makes no sense to facilitate arbitrary modulation in the constructor.
I do think showing an example of multiplication by a scalar field is important. Perhaps using it to return the polarization field would be a better example. Do you agree?
Best,
Stephen
I do think showing an example of multiplication by a scalar field is important. Perhaps using it to return the polarization field would be a better example. Do you agree?
Yeah, why not, it's hard to come up with a use case but we can show it. Maybe multiplication by a narrow Gaussian mimicking the wave function of a finite-sized quantum dot?
Hi all,
I wanted to let you know that I likely won't return to this until later in December. As I am completing my applications to graduate schools and writing finals.
Best,
Stephen