Skip to content

add dispersion model for comparison and use a few new API improvements #297

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

Merged
merged 1 commit into from
Jun 18, 2025

Conversation

dmarek-flex
Copy link
Contributor

@dmarek-flex dmarek-flex commented May 27, 2025

Big change is adding the dispersion model from scikit-rf. I also changed to a 3D PEC for the strip, since our conformal meshing is much improved.

Also added section describing the different definitions of impedance commonly used.

  • Need to rerun with the next version of the solver (2.8.4 has some bugs which cause the notebook to error)

Copy link
Contributor

github-actions bot commented May 27, 2025

Spell Check Report

CharacteristicImpedanceCalculator.ipynb:

880: "[4]   R. H. Jansen and M. Kirschning, “Arguments and an accurate Model for the Power-Current Formulation of Microstrip Characteristic Impedance”, Archiv für Elektronik und Übertragungstechnik (AEÜ), vol. 37, pp. 108-112, 1983.\n",
	Archiv ==> Archive

Checked 1 notebook(s). Found spelling errors in 1 file(s).
Generated by GitHub Action run: https://github.com/flexcompute/tidy3d-notebooks/actions/runs/15742913183

@dmarek-flex dmarek-flex linked an issue May 27, 2025 that may be closed by this pull request
@dmarek-flex dmarek-flex force-pushed the dmarek/update_impedance_notebook branch 2 times, most recently from 6eeafe7 to 19880a5 Compare May 28, 2025 17:41
@daquinteroflex
Copy link
Collaborator

Looks good! Would it be possible to use the skrf prefix instead of rf because that way it's easier to work out what's external to tidy3d since rf and our microwave would appear to be from the same package.

Copy link
Contributor

@weiliangjin2021 weiliangjin2021 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

  • "Internally, the ImpedanceCalculator computes impedance using ..." maybe change it to mention that it is one of the 3 approaches
  • For Gaussian pulse, how is the result if it's constructed from from_frequency?
  • Could you comment how the 3 methods compared to skrf result?

@dmarek-flex
Copy link
Contributor Author

For Gaussian pulse, how is the result if it's constructed from from_frequency?

Basically no change

@dmarek-flex
Copy link
Contributor Author

dmarek-flex commented Jun 6, 2025

Could you comment how the 3 methods compared to skrf result?

Done, I think I found a better way to compare the methods and explain the differences when compared to scikit-rf. I want to still finalize the last cells to show a calculation of a lossy transmission line.

@weiliangjin2021
Copy link
Contributor

For Gaussian pulse, how is the result if it's constructed from from_frequency?

Basically no change

In that case, better to use from_frequency to promote this approach?

@dmarek-flex dmarek-flex force-pushed the dmarek/update_impedance_notebook branch from e43b06c to 5edf0ce Compare June 6, 2025 20:35
@dmarek-flex
Copy link
Contributor Author

For Gaussian pulse, how is the result if it's constructed from from_frequency?

Basically no change

In that case, better to use from_frequency to promote this approach?

Ah, ok! It did change the FieldTimeMonitorData, so I'll need to edit those plots.

@dmarek-flex dmarek-flex force-pushed the dmarek/update_impedance_notebook branch 2 times, most recently from d9d5cc2 to 63f0634 Compare June 9, 2025 18:49
@tylerflex
Copy link
Collaborator

should we merge this?

@dmarek-flex
Copy link
Contributor Author

dmarek-flex commented Jun 17, 2025

should we merge this?

I was waiting for the next version of the solver to rerun, so I can remove the temporary solver_version. I think I can do that with the current pre release.

@dmarek-flex dmarek-flex force-pushed the dmarek/update_impedance_notebook branch from 63f0634 to da873ab Compare June 18, 2025 20:24
switched scikit rf comparison to PI model and directly compared with the three definitions available in Tidy3d

added comparison for a lossy transmission line mode
@dmarek-flex dmarek-flex force-pushed the dmarek/update_impedance_notebook branch from da873ab to fd2041a Compare June 18, 2025 20:26
@dmarek-flex dmarek-flex merged commit d0e4871 into develop Jun 18, 2025
1 check passed
@dmarek-flex dmarek-flex deleted the dmarek/update_impedance_notebook branch June 18, 2025 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update impedance notebook to apply a frequency-dependent model
4 participants