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

Store all signal and improve structuce of signal line #29

Closed
fedroy opened this issue Jan 29, 2021 · 3 comments · Fixed by #129
Closed

Store all signal and improve structuce of signal line #29

fedroy opened this issue Jan 29, 2021 · 3 comments · Fixed by #129
Assignees
Labels
can-wait Not working, not urgent enhancement New feature or request good first issue Good for newcomers
Milestone

Comments

@fedroy
Copy link
Collaborator

fedroy commented Jan 29, 2021

At the moment, most devices have a self.signal where the last signal that was produced gets stored.
For example:

c3/c3/generator/devices.py

Lines 926 to 931 in 815bf32

cos = tf.cos(omega_lo * ts)
sin = tf.sin(omega_lo * ts)
self.signal["inphase"] = cos
self.signal["quadrature"] = sin
self.signal["ts"] = ts
return self.signal

We should instead store all signals generated in the generator object by doing something like

signal_stack: List[tf.Variable] = []
for dev_id in self.chains[chan]:
    dev = self.devices[dev_id]
    ...
final_signal[chan] = copy.deepcopy(signal_stack.pop())
signal_stages[chan] = signal_stack

For this to work we would have to make sure that when taking elements from the signal stack they aren't popped.

However, this creates a problem:
Imagine your generator stack has the following devices:
LO, LO_noise, AWG, AWG_noise, Mixer
where the LO_noise and AWG_noise take 1 input and have 1 output.
Then the popping works because the mixer will find in the stack the noisy AWG output and the noisy LO output.
If we don't pop the stack when we get to the mixer would have the outputs:
[LO, LO_noise, AWG, AWG_noise]
And won't know that it needs to take the 2nd and 4th elements of the stack.

This leads to the point that we need to implement a more general (and more intuitive) signal generation chain that is a directed graph.
One way to do this would be when specifying the chain you specify the inputs.
What is currently:

chains={
        "TC":["lo", "lo_noise" , "awg", "dac", "resp", "mixer", "fluxbias"],
        "Q1":["lo", "awg", "dac", "resp", "mixer", "v2hz"],
    } 

could become:

chains={
        "TC": [("lo"), ("lo_noise","lo") , ("awg"), ("dac","awg"), ("resp","awg"), ("mixer","lo_noise","resp"), ("fluxbias", "mixer")]
        "Q1": [("lo") , ("awg"), ("dac","awg"), ("resp","awg"), ("mixer","lo","resp"), ("v2hz", "mixer")]
} 

where the first element of each tuple is the device that needs to make a signal and the others are the inputs.
This is to get closer to the point where we truly have a directed graph.
The tuple structure is just a suggestion but anything would do even:

chains={
        "TC": [
            {"device": "lo", "inputs" : []},
            {"device": "lo_noise", "inputs" : ["lo"]} ,
            ...,
            {"device": "mixer", "inputs" : ["awg", "lo"]},
            ...
        ]   
        "Q1": [{...}, ..., {...}]
} 

I'm aware this is a more annoying to setup but it does make more sense to someone that isn't familiar with the stack and how to make it work for new devices.

[thanks @GlaserN for the input]

@fedroy fedroy added enhancement New feature or request good first issue Good for newcomers labels Jan 29, 2021
@nwittler
Copy link
Collaborator

I think there should be some discussion whether we want to store all of this in the object. We already do this in a number of places, the propagation too for example, and I suspect this bloats the runtime environment. Better practice might be to get good logging in general. And even then, storing absolutely everything is a debug situation and not required in production, or is it?

@lazyoracle lazyoracle added the can-wait Not working, not urgent label Feb 2, 2021
@alex-simm
Copy link
Collaborator

Looks to me like there are two separate things:

  • allow obtaining the signal in between devices in the line: I'm guessing this would be mainly for debugging. For this, a callback that is called after each device might be the best option. It doesn't create any overhead and allows people to retrieve and store the signals if needed.
  • improving the signal line structure to something more tree-like: instead of passing a large 'chains' dictionary to the generator, I would propose connecting the devices in an object-oriented way. Each device can store its successor (or predecessors) and the generator only needs to iterate through that tree.

@shaimach
Copy link
Collaborator

shaimach commented Jul 14, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
can-wait Not working, not urgent enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants