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

WIP : Noisy network and quantum limits #401

Draft
wants to merge 70 commits into
base: master
Choose a base branch
from

Conversation

rdireen
Copy link

@rdireen rdireen commented Oct 14, 2020

Purpose

My friend and I added the ability to combine scikit-rf Networks in a way that correctly handles noise covariance calculations. Meaning you can calculate noise parameters (nfmin, y_opt, nf, etc.) after combining Networks in various ways: cascade, parallel-parallel, series-series.

In addition, the calculations handle quantum limits at very cold and/or high frequencies. This code is being used to analyze networks that are cryogenically cooled to temperatures near 0 K. With our additions you should be able to play with and design LNAs.

Examples

You can see our examples under doc/examples/noisynetworks or you can compile the sphinx documentation and look under Examples/Network Noise Analysis. The best examples to start with are "Noise Analysis of Common Emitter Amplifier with Two-Port Networks" and "Multiport Networks Exposed to Multiple Temperatures and Quantum Behavior."

ToDo

This is our initial pull request to see what you all think, but there are additions and improvements we would like to make. We have only started in on unit tests, so we haven't adequately covered the code. We also acknowledge that we need more documentation. But, we have added some examples and have done a first pass of API documenting.

Please let me know what you think, we would love to collaborate with you on your excellent project! The e-mail associated with my GitHub account would work fine for any questions.

rdireen and others added 30 commits August 4, 2020 18:55
This is a first step to provide scikit-rf with some additional noise processing capabilities. I have basic functionality for cascading two port networks together in a way that enables the user to calculate the overall noise parameters of the combined two-ports. Next I will add the ability to combine two-ports in parallel and in series.
Results calculated with the new NetworkNoiseCov object are compared to analytic results from Fukui's paper on modeling noise within a BJT
Given a number of multi-port networks, this code will allow you to connect them to get a final network that correctly calculates the overall noise covariance matrix
Given a number of multi-port networks, this code will allow you to connect them to get a final network that correctly calculates the overall noise covariance matrix
Also updated simple_noise_cascade.ipynb to reflect the attenuator up update
…a-garcia multi port thermal noise and NF calculator.
The small signal model is the same that is used in SPICE (at least I believe base on some less than optimal research). I added an example ipynb that describes the model and what is happening on the inside. The model handles thermal and shot noise.
Also working on getting multi os development cleaned up
skrf/network.py Outdated Show resolved Hide resolved
skrf/network.py Outdated Show resolved Hide resolved
skrf/network.py Outdated Show resolved Hide resolved
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.

My general comment is that there is a big work here so first of all, thank you very much for that.

To the form, since most of the files of the PR haven't been changed (whitespace / end of lines), I would recommend to not include them in the PR and to let them unchanged.

I would also suggest delaying the addition of the noisyComponents/*.py for a future PR : we must first address the NoisyNetwork and its associated networkNoiseCov to get consistent with Network API. (and logical for users). For example, if the operators + and / have a different meaning between Network and NoisyNetwork, this may lead to future issues.

Please find my comments attached and don't forget this is only my point of view and can be of course discussed.

skrf/network.py Show resolved Hide resolved
skrf/network.py Outdated Show resolved Hide resolved
skrf/network.py Show resolved Hide resolved
skrf/network.py Outdated Show resolved Hide resolved
skrf/network.py Show resolved Hide resolved
Comment on lines +4 to +9
from .attenuator import *
from .circulator import *
from .rlc_series_2port import *
from .rlc_shunt_2port import *
from .bjt_ce_2port import *
from .wilkinson_nport import *
Copy link
Member

Choose a reason for hiding this comment

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

I would recommand to postpone the noisyComponents in a future PR, the time for noisyNetwork API to be integrated.

On the API point of view, we need to discuss (@arsenovic ?). I see two things:

  • does it have sense to have a bunch of new classe names, but knowing that these classes are in fact all Network or NoisyNetwork objects.
  • If there is an interest in doing so, we could be more ambitious: Instead of a directory noisyComponents/, maybe it is worth creating a components/ directory, with some noisy components inside (or as a subdir)

Copy link
Author

Choose a reason for hiding this comment

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

What I can do (at least temporarily) is place noisyComponents within the doc/examples/noisynetworks. That way all our examples will work, but we don't have to make noisyComponents a part of the core scikit-rf code. Does this sound good to you?

As far as adding new class names, I believe this is within the spirit of object oriented programing. It is true that all of these new classes are Networks or NoisyNetworks, in the same way that a dog or a cat is an animal. For example, you might make an animal class have 4 legs, a mouth and ears, but the derived dog class might add the ability to bark and the derived cat class have whiskers. An attenuator and a circulator are both Networks, so the natural object oriented mechanism for creating these devices in code is to derived them from Network using inheritance.

However, that's more philosophy than anything. You could of course just create a Network and name it an attenuator. Since these decisions are more up to you and the other creators of scikit-rf, I will happily defer to your judgement.

As far as an opinion goes, I think it would be nice to have a folder called components where all objects are derived from NoisyNetwork. Since NoisyNetwork just extends Network, all the functionality that you would expect from Network would be transferred to the derived component.

Comment on lines +118 to +119
\+ combines noisy networks in series
\| combines noisy networks in parallel
Copy link
Member

Choose a reason for hiding this comment

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

these operators have a different meaning in Network. I'm afraid this be a source of future confusion and issues

Copy link
Author

Choose a reason for hiding this comment

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

I agree. Would you be willing to permit adding a global flag that would enable changing the functionality? I did this before and it worked well. Basically you do something like rf.enable_new_operators() at the top of a script and it changes the functionality of the operators. What I like about using operators with two-port networks is that you can create an entire circuit and the result just looks like a line of algebra. This is far more convenient that having to call individual functions for each combination.

But, again the ball is in your court. If you think this is too awkward I can be convince to drop this functionality. Or perhaps use different operators (if there are any left :D).

Copy link
Member

Choose a reason for hiding this comment

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

It's an interesting feature, Is it working for Nport as well ?


"""

def __init__(self):
Copy link
Member

Choose a reason for hiding this comment

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

passing a list of NoisyNetwork to the constructor would be convenient.


return enumb, errors

def reduce(self):
Copy link
Member

Choose a reason for hiding this comment

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

I didn't go into the implementation, but isn't it equivalent to cascade multiple NoisyNetwork together :

reduced_nntwk = nntwk1 ** nntwk2 ** ... ** nntwkN

?

Copy link
Author

Choose a reason for hiding this comment

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

No, the point of multiNoisyNetworkSystem.py is to work with multiport NoisyNetworks. For example, you can combine multiport splitters and three port circulators with this object in any combination. The reduce() function returns a new NoisyNetwork that is not necessarily a two-port.

This IS very similar to your circuit.py module. The problem is that I don't currently know how to combine noise covariance matrices in a way that works with your circuit.py code. But, one of the things @mgrando needs in his work is the ability to combine multiport noisy networks. So I was kinda stuck and needed a mechanism that would do something similar to circuit.py but work with NoisyNetworks. I'm open to suggestions if you have any ideas on how to perhaps make this a part of circuit.py

Copy link
Member

Choose a reason for hiding this comment

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

ok, see my comment below

from .noisyNetwork import NoisyNetwork
from .networkNoiseCov import NetworkNoiseCov

class MultiNoisyNetworkSystem(object):
Copy link
Member

Choose a reason for hiding this comment

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

Before adding a new class to the skrf API (which will be a future source of issues ;), I'm asking myself "it is necessary?"

If I understand well, a MultiNoisyNetworkSystem , If only made of 2ports, consists in a multiple cascaded NoisyNetworks, am I right ?
What if some NoisyNetwork in the list have more ports than 2 ?
It sounds like a kind of Circuit for NoisyNetworks

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, see my above comment. MultiNoisyNetworkSystem works with multi-port NoisyNetworks. So more ports than two-ports. I made a lousy example that only worked with two-ports, I should have made something with splitters and circulators or somethings.

This is very much like Circuit for NoisyNetworks.

Copy link
Member

Choose a reason for hiding this comment

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

OK, I understand.

For the sake of clarity, why not calling it NoisyCircuit then ? OK, the way of defining such circuit is a bit different from Circuit's way, but the purpose looks like identical.

From the theoretical point of, it would be interesting to see if the Circuit's building algorithm taken from (Hallbjörner, P., 2003, . Microw. Opt. Technol. Lett. 38, 99–102. https://doi.org/10/d27t7m ) could be extended to noisy Network as defined in Garcia's 2004 paper.

@rdireen
Copy link
Author

rdireen commented Nov 29, 2020

@jhillairet thank you very much for spending the time to review these potential additions. I can't get to the code today but will try to put some time in this week. I've read your comments and I am in general agreement with you. And of course you know skrf better than I so I greatly value your comments.

I will hopefully be providing you with updates soon.

@rdireen
Copy link
Author

rdireen commented Nov 30, 2020

@jhillairet I've cleaned up the PR and addressed many of your comments. I'll get back to working on the rest of them this week. Thanks again for taking the time to consider our additions.

@jhillairet
Copy link
Member

jhillairet commented Nov 30, 2020

@jhillairet I've cleaned up the PR and addressed many of your comments. I'll get back to working on the rest of them this week. Thanks again for taking the time to consider our additions.

thanks, I'll check it. We can also wait for other's advices, which maybe differ from mine.

@arsenovic
Copy link
Member

if all the new additions are modular, could this be re-submitted easily as a fresh PR? just for the sake of readability?

@rdireen
Copy link
Author

rdireen commented Dec 14, 2020

@jhillairet and @arsenovic. Sorry for the delay, looks like December is going to be buys for me. After the new year I'll get back to work on this stuff. I'll fix some more of the pieces @jhillairet suggested and start a new pull request.

@jhillairet jhillairet changed the title Noisy network and quantum limits WIP : Noisy network and quantum limits Jan 8, 2021
@jhillairet
Copy link
Member

Hi @rdireen

Do you plan to continue your work on this? There is definitely a need for dealing with noisy networks (cf #538 for example)

@rdireen
Copy link
Author

rdireen commented Oct 20, 2021

Hi @jhillairet

I would definitely like to continue working on the noisy networks project (I know @mgrando would too). It is nice to know that there is a need out there for these features. I am currently busy with work, but I might be able to jump into getting things together in November.

You may be interested to know that @mgrando has verified much of the code we have written via experiment. The code looks like it correctly handles NF near 0 degrees K.

@jhillairet jhillairet marked this pull request as draft May 11, 2022 15:24
@jhillairet
Copy link
Member

Update : the work made in this branch by @rdireen and @mgrando has been used in the following paper https://arxiv.org/pdf/2209.04008.pdf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvements Improvements of existing feature New Feature A new feature in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants