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

Vector Fitting: Bug fix for #677 and #702 #678

Merged
merged 20 commits into from
Jun 12, 2022
Merged

Conversation

Vinc0110
Copy link
Collaborator

@Vinc0110 Vinc0110 commented May 22, 2022

Fixes issue #677 (and #702).
Also a warning is printed in case the passivity enforcement did not succeed (when n_samples was too small).

The problem was an incorrect choice of the evaluation frequencies for the passivity evaluation.

Quote from the paper [1]:
First, a dense set of frequencies is determined from dc up to about 20% above the highest relevant frequency. This highest relevant frequency is the maximum of the highest crossing from a nonpassive to a passive region on one hand and the maximum frequency of interest on the other hand.

The second condition was missing in the code.

[1] T. Dhaene, D. Deschrijver and N. Stevens, "Efficient Algorithm for Passivity Enforcement of S-Parameter-
Based Macromodels," in IEEE Transactions on Microwave Theory and Techniques, vol. 57, no. 2, pp. 415-420,
Feb. 2009, DOI: 10.1109/TMTT.2008.2011201.

@Vinc0110 Vinc0110 linked an issue May 22, 2022 that may be closed by this pull request
@Vinc0110 Vinc0110 marked this pull request as draft May 22, 2022 14:47
@Vinc0110
Copy link
Collaborator Author

So I fixed one part, but I'm not sure if this really fixes all the problems. It still seems a bit strange for certain setups. I need to check it some more.

@jhillairet jhillairet added this to the v0.23.0 milestone Jun 4, 2022
@Vinc0110
Copy link
Collaborator Author

Vinc0110 commented Jun 11, 2022

This PR ended up as a major revision of the passivity enforcement routine. It now works much better and the routine offers a bit more control with the new parameters n_samples and f_max. Issue #677 is solved for n_samples=500.

I'll leave it for now and will merge soon, if no one complains.

@Vinc0110 Vinc0110 marked this pull request as ready for review June 11, 2022 18:15
@Vinc0110 Vinc0110 marked this pull request as draft June 12, 2022 09:15
@Vinc0110 Vinc0110 changed the title Vector Fitting: Bug fix for #677 Vector Fitting: Bug fix for #677 and #702 Jun 12, 2022
@Vinc0110
Copy link
Collaborator Author

Vinc0110 commented Jun 12, 2022

Passivity assessment and enforcement now works for most of my test cases. Enforcement still sometimes fails if the fit is terribly non-passive, which is probably a natural limitation. A massive number of samples would be required, which quickly becomes impractical. A UserWarning is issued in those cases, suggesting to re-fit the model with fewer poles and/or without the constants.

@Vinc0110 Vinc0110 marked this pull request as ready for review June 12, 2022 13:38
@Vinc0110 Vinc0110 merged commit b370b97 into scikit-rf:master Jun 12, 2022
@Vinc0110 Vinc0110 deleted the fix677 branch June 12, 2022 16:24
@jhillairet jhillairet mentioned this pull request Jun 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants