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

Add noise data to write_touchstone #906

Merged
merged 3 commits into from
May 22, 2023
Merged

Conversation

ericwrice
Copy link
Contributor

Quick addition to write noise data to touchstone files as raised in #379. This follows the touchstone 1.1 specification. Tested by reading in the following transistor touchstone and writing out the test file.

LN300.s2p.txt
test.s2p.txt

@jhillairet jhillairet added Touchstone concerning Touchstone formats reading/writing New Feature A new feature in progress labels May 2, 2023
Copy link
Member

@jhillairet jhillairet left a comment

Choose a reason for hiding this comment

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

LFTM.

Eventually if you could add associated tests?

@ericwrice
Copy link
Contributor Author

Yep, good idea, I'd be happy to add some tests. That made me notice it works fine with the file I used that had equal length scattering and noise parameters, but not with the noisy networks in the test folder that have different lengths.

It looks to me that the original noise information is not stored in the network, only the interpolated data. Do we want write_touchstone() to use the interpolated data or find a way to maintain the original?

Probably a separate issue, but noise interpolation in Network.n fails on skrf/tests/ntwk4_n.s2p because the lowest noise frequency is less than the lowest s frequency, even though this is allowed in the touchstone specification. It's probably good practice to avoid extrapolating outside of provided data, but could make some valid touchstone files more difficult to work with.

@ericwrice ericwrice marked this pull request as draft May 4, 2023 22:08
@jhillairet
Copy link
Member

It looks to me that the original noise information is not stored in the network, only the interpolated data. Do we want write_touchstone() to use the interpolated data or find a way to maintain the original?

I have no advice on this. Anyone?

@ericwrice
Copy link
Contributor Author

I decided it was best to only write the original data for now. That way measured data doesn't get mixed with interpolated data for example. Also added a test.

@ericwrice ericwrice marked this pull request as ready for review May 10, 2023 16:03
@jhillairet jhillairet merged commit b9e1e78 into scikit-rf:master May 22, 2023
1 check passed
@jhillairet
Copy link
Member

Thanks !

@jhillairet jhillairet mentioned this pull request Jun 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Network New Feature A new feature in progress Touchstone concerning Touchstone formats reading/writing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants