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

Duffing system model generators for pulse simulator #516

Merged

Conversation

DanPuzzuoli
Copy link
Contributor

Summary

Functionality requested in issue #514 . A user can now construct and simulate a transmon system by only specifying a list of frequencies and couplings, without any reference to the string format.

  • Main addition is the file transmon_model_generators.py, containing functionality for generating PulseSystemModel objects for transmon system models
  • Tests for all functions in transmon_model_generators.py have been added in test_transmon_model_generators.py
  • A new example notebook as been added: example/transmon_model_example.ipynb, which demonstrates the usage. (This is for internal viewing of how it works, and also to do sanity check that some of the simulations in example/pulse_sim.ipynb work as expected.)

Details and comments

The only "public" function in transmon_model_generators.py is transmon_system_model, though there are a variety of functions that it builds on (that may be of later use). To create the system model, the user needs to specify:

  • number of transmons
  • dimension of transmons
  • transmon frequencies, anharmonicities, drive strengths
  • a "coupling graph", i.e. a specification of what qubits are coupled, along with the coupling strengths
  • dt (the sampling rate for pulses)

The function returns a tuple with:

  • A PulseSystemModel object representing the specified system, that can be plugged into the simulator
  • a dict, which in the code is called cr_idx_dict (short for cross-resonance index dict). This dict stores u channel index information for doing cross-resonance pulses. E.g. to drive a CR pulse on qubit q_driven (an int) with target qubit q_targ, use the u channel with index cr_idx_dict[(q_driven, q_targ)].

…iform couplings, though still need to add u_channel_lo construction functions
@DanPuzzuoli
Copy link
Contributor Author

Note: previous attempts at tests were due to an error being raised in assemble in one of the pulse simulator tests. This error wasn't occurring on my machine locally, even after updating to the latest version of the master branch of terra (not sure if I made a mistake here).

The test was failing due to the pulse simulator DEFAULT_CONFIGURATION not having meas_levels. I added this as { 'meas_levels': [0,1,2]} and it seems to work fine.

@chriseclectic chriseclectic added this to the Qiskit Aer 0.4 milestone Jan 21, 2020
@haggaila
Copy link

Hi, if the code implements a nonlinear oscillator with a quartic nonlinearity (proportional to n^2 with n = a^\dag a), it may be more precise to refer to it as a "Duffing" (or "quartic") oscillator, as it is an approximation of a transmon model with a cosine-type nonlinearity, which may be implemented in the future.

@chriseclectic
Copy link
Member

@haggaila, yes you are right, I think we should rename this to duffing oscillator model

@DanPuzzuoli DanPuzzuoli changed the title Transmon system model generators for pulse simulator Duffing system model generators for pulse simulator Jan 21, 2020
@DanPuzzuoli
Copy link
Contributor Author

I've converted all relevant files to use nomenclature relating to "duffing oscillator" instead of "transmon"

Copy link
Member

@chriseclectic chriseclectic left a comment

Choose a reason for hiding this comment

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

This is getting there, but I think the signature of the function could be further simplified.

Something like:

duffing_system_model(
   oscillator_dims,
   oscilator_freqs,
   anharm_freqs,
   coupling_freqs,  # list [((i, j), J_ij), ...]
   drive_strengths=None, # If None should this have default of [1, 1, 1, ...]?
   dt=1)

I'm also not sure what the units are for the values. Are oscillator_freqs entered in angular or frequency units? (ie will they be multiplied by 2*pi later or not), are they assumed to be in Hz/GHz etc?

qiskit/providers/aer/openpulse/duffing_model_generators.py Outdated Show resolved Hide resolved
qiskit/providers/aer/openpulse/duffing_model_generators.py Outdated Show resolved Hide resolved
qiskit/providers/aer/openpulse/duffing_model_generators.py Outdated Show resolved Hide resolved
* ``{(0,1): 0.02, (1,3): 0.01}`` specifies a system in which
oscillators (0,1) are coupled with strength 0.02, and (1,3) are
coupled with strength 0.01
dt (float): Pixel size for pulse instructions
Copy link
Member

Choose a reason for hiding this comment

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

Can dt have a default value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is related to the units. I lean towards no but we can discuss this.

coupling_symbol (str): Coupling symbol for internal HamiltonianModel

Returns:
tuple[PulseSystemModel, dict]: The generated oscillator system model, and a dict specifying
Copy link
Member

Choose a reason for hiding this comment

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

returning the idx as well as the pulse model seems messy. It makes me think that the coupling channel should be a method of the PulseSystemModel itself

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue is that U channels are completely arbitrary; in this case we are using them for a particular type of two-qubit interaction, but in general they can be anything. As a result I'm not sure this index information should be put into PulseSystemModel, as the meaning of the identifying information (the ordered qubit pair) is strongly tied to the particulars of this model.

The options I can think of are:

  1. Put the information into PulseSystemModel: I think to maintain generality of this class we would need to add a general dictionary to it for control channel index info, where the type of the keys is arbitrary, so that they can contain identifying info for channel indices of any format, to not make assumptions about the model.
  2. Define a new class called DuffingSystemModel that inherits from PulseSystemModel, but that stores cr_idx_dict, and has an added method for looking things up in it. I do kind of like this option as it seems like a natural thing to do when more functionality is needed beyond simply containing info necessary for running the simulation.
  3. Leave it as is. I think for now I prefer this option as it's pretty simple, and we can think more about what the PulseSystemModel should be.

For example:

* Given the coupling_dict ``{(0,1): 0.02, (1,3): 0.01}``,
the returned cr_idx_dict will be:
Copy link
Member

Choose a reason for hiding this comment

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

If order matters for assigning channels, maybe the couping input should be a list [[(i,j), val), ...], where list-position becomes channel number, rather than a dict

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue here is that for the purpose of specifying couplings there should be no difference between (0, 1) and (1, 0); they should both just mean that qubits 0 and 1 are coupled. As such if we want the user to specify u channel orders themselves it should be in a separate argument (as otherwise val will need to be duplicated for both (0,1) and (1, 0)). In terms of method signature/output it would be simpler to make the user specify u channel orderings themselves, but in terms of actual use it will be more cumbersome.

It's somewhat annoying to deal with as for u channels, consistency in order matters, but the ordering itself is completely arbitrary.

"""Creates a hamiltonian string dict for a duffing oscillator model

Note, this function makes the following assumptions:
- oscillators, oscillator_dims, oscillator_freqs, freq_symbols, anharm_freqs,
Copy link
Member

Choose a reason for hiding this comment

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

As with above function assumption means oscillators is unnecessary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here oscillators is a list containing arbitrary indices.

# Helper classes


class _coupling_graph:
Copy link
Member

Choose a reason for hiding this comment

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

Class should be capitalized class CouplingGraph

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to keep this private, how is _CouplingGraph?

dim_oscillators (int): Dimension of truncation for each oscillator
oscillator_freqs (list): Oscillator frequencies, assumed to be of length num_oscillators
anharm_freqs (list): Anharmonicity values, assumed to be of length num_oscillators
drive_strengths (list): Drive strength values, assumed to be of length num_oscillators
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if this is a frequency or a scale value? What does its value mean relative to oscillator_freqs for example (ie is it same units)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oscillator_freqs, anharm_freqs, and drive_strengths are all the same units. As described in the doc string, the drive_strengths elements enter as coefficients in the Hamiltonian. I called it drive_strengths instead of drive_freqs as 'drive freq' is potentially confusable with the lo frequency.

@DanPuzzuoli
Copy link
Contributor Author

  • With regards to units:
    • there is no assumed timescale. Nothing in the simulator cares if it's GHz/ns or Hz/s; so long as the units are consistent, it will output the same results regardless of the units used. We could write in the doc string that we are assuming some units, but it seems odd to do so given that it has no effect on the behaviour.
    • Also, frequencies are radial, i.e. everything gets multiplied by 2*pi. The multiplication by 2*pi is described in the doc string, but it is worth adding a point right at the beginning to say that everything is assumed radial.
  • With regards to defaults:
    • I would prefer to not have default values, as 'natural' values for one parameter will depend on what the other parameters are, and it's not clear to me what value will be provided for the added complication of figuring these things out.

@DanPuzzuoli
Copy link
Contributor Author

As discussed, the changes are:

  • Added a dictionary PulseSystemModel.control_channel_dict and a function PulseSystemModel.control_channel_index that queries the dictionary (and provides warnings if a key is non-existent).
  • updated duffing_system_model to only return a PulseSystemModel
  • changed _coupling_graph to CouplingGraph
  • updated all relevant test cases to reflect these changes
  • updated the model generator example

Copy link
Member

@chriseclectic chriseclectic left a comment

Choose a reason for hiding this comment

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

LGTM!

@chriseclectic chriseclectic merged commit 58d1059 into Qiskit:openpulse-sim Jan 23, 2020
chriseclectic pushed a commit to chriseclectic/qiskit-aer that referenced this pull request Jan 23, 2020
* Created new file for helper functions for creating HamiltonianModel and PulseSystemModel objects for duffing oscillator systems

* increased max shots in pulse_simulator

* added PulseSystemModel.control_channel_index() along with tests
chriseclectic pushed a commit to chriseclectic/qiskit-aer that referenced this pull request Jan 23, 2020
* Created new file for helper functions for creating HamiltonianModel and PulseSystemModel objects for duffing oscillator systems

* increased max shots in pulse_simulator

* added PulseSystemModel.control_channel_index() along with tests
@DanPuzzuoli DanPuzzuoli deleted the pulse-sim-model-generators branch June 4, 2020 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants