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

CurrentDetector and FFT progress #27

Merged
merged 37 commits into from
Oct 3, 2021
Merged

CurrentDetector and FFT progress #27

merged 37 commits into from
Oct 3, 2021

Conversation

0xDBFB7
Copy link
Contributor

@0xDBFB7 0xDBFB7 commented Jul 22, 2021

Hi!

This is a little more towards #9. I think FrequencyRoutines.FFT() might perhaps cover #1 when it's done (prerequisite).

You can merge #23 now if desired; this PR is also unfinished, you can review later if you want.

It's such a simple procedure, (apply a pulse, measure the voltage and current at each node, FFT both V and I,
divide the two spectra to get Z-param, multiply and divide conjugates to get S-param), I'm just taking a while to implement it.

Several parts might not be what you're looking for, feel free to suggest API improvements:

  • New functions only work on a single cell in the Detector. Haven't figured out how one should "sum" E-detector cells over a certain Block for S-param purposes (averaging?). Maybe relates to Method for calculation of average power #25. CurrentDetector only reports one polarization, to be fixed.
  • Some holdouts still formulated in terms that make sense in the microwave systems I've been working with (specifically, SoftArbitrary taking a "voltage" amplitude input, and the antenna test case), but less so for the optical domain that /fdtd/ seems to mostly be used for.
  • Varying magnetic permeability has not been accounted for in the current detector, because I don't have any test cases for that and I'm not well-versed enough to figure it out from first principles :P

Thanks for your patience :)

@0xDBFB7 0xDBFB7 changed the title CurrentDetector progress, CurrentDetector and FFT progress Jul 22, 2021
@flaport
Copy link
Owner

flaport commented Jul 22, 2021

This is amazing! I'll start reviewing this asap! (might take me a while though...)

Short answers to the points you brought up:

  • I think that's indeed related to Method for calculation of average power #25
  • That's fine, we can work on that later.
  • Varying magnetic permeability is pretty rare in purely optical materials and I would leave it to the people who actually need those features to implement that themselves ;-). That said, maybe you can check for a varying permeability before doing the calculations and raise a NotImplementedError or a warning if a varying permeability is present.

@flaport flaport merged commit 62685f6 into flaport:dev Oct 3, 2021
@flaport
Copy link
Owner

flaport commented Oct 3, 2021

Hi @0xDBFB7 , thanks for this PR. What you've added so far looks good to me! I've merged it. If there are any new changes you'd like to add feel free to open a new PR 🙂

Also, would you be willing to add an example notebook going over the new features you've added?

@0xDBFB7
Copy link
Contributor Author

0xDBFB7 commented Oct 3, 2021

@flaport Wonderful! Thanks for the review and ping. I'll work on a notebook.

@flaport
Copy link
Owner

flaport commented Oct 11, 2021

Thanks, @0xDBFB7 ,

I also saw you left some comments in the PR with some good ideas! I'll leave a comment here if I implement any of them.

Currently implemented:

  • fp16 (numpy and torch) and fp128 (numpy only) support

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.

None yet

2 participants