Adding PulseSystemModel class as higher level class containing HamiltonianModel #496

DanPuzzuoli · 2019-12-13T19:22:34Z

Update

This is no longer a [WIP] - and is ready for review.

Summary

This is a [WIP], but the purpose of the new class SimSystemModel is to:

Along the lines of the recent HamiltonianModel addition, detangle the options specifications for a pulse simulation. It contains a HamiltonianModel instance, along with any other remaining model information not naturally contained in HamiltonianModel, such as the sampling rate dt and u_channel_lo.
The place that users can manipulate/probe the properties of the model; along with HamiltonianModel, this is where model-based functions like frequency calculations can live that the user can use.

To instantiate from a backend:

from qiskit.providers.aer.openpulse.pulse_system_model import PulseSystemModel

backend_real = FakeOpenPulse2Q()
system_model = PulseSystemModel.from_backend(backend_real)

To simulate using this model, and an already assembled pulse qobj:

sim_result = backend_sim.run(qobj, system_model, backend_options=backend_options).result()

Now, the argument backend_options will contain only information specific to the simulator backend, like ODE options (as opposed to a combination of model and solver parameters as before).

Details and comments

I still need to make the following changes, but comments are welcome.

Change tests to use this object
Change example/calibration notebooks to use this object
Write docs for new functions,

I'm not crazy about the name SimSystemModel, any suggestions @chriseclectic @atilag ? SystemModel sounds better, but I thought it might be too general.

…to simulation

…ate_channel_frequencies

…ils.__init__ for pulse_sim_utils

…ngs to HamiltonianModel

…alculate_channel_frequencies

…SimSystemModel

…ication from string parsing

…field

DanPuzzuoli · 2019-12-15T03:23:58Z

Update: Made some changes, but in terms of the to do list, migrated the digest test functions into a new file for testing behaviour of model classes.

There is a slight issue that will cause things to break at some point: terra now reports defaults in s/Hz, but some units in configuration for a backend seem to still be in GHz, including reporting of the Hamiltonian. This inconsistency makes things a bit awkward still; e.g. currently for things to work it's necessary to specify system_model.dt = 1 after constructing from a backend, as the Hamiltonian parameters in configuration has units of GHz, but configuration['dt'] is in seconds.

Edit: This inconsistency is apparently discussed in either a terra issue or PR, so should be changed at some-point soon, which will break the tests here, but I figure the only thing we can do is fix it when it happens does.

…rack

DanPuzzuoli · 2019-12-18T19:04:43Z

The latest updates are described by the previous comment; it has been edited as it is mostly the same. The only edit is the removal of the methods from PulseSystemModel that behave like a pulse backend configuration; when constructing schedules the current paradigm is for the user to just directly call pulse.DriveChannel(0) to reference the drive channel on qubit 0.

…default memory_slots setting in digest to be equal to the number of qubits

chriseclectic

Initial review. My main question is do both classes need channel information? Currently this seems to be duplicated in both classes.

qiskit/providers/aer/backends/pulse_simulator.py

qiskit/providers/aer/openpulse/hamiltonian_model.py

chriseclectic · 2019-12-19T15:32:06Z

qiskit/providers/aer/openpulse/hamiltonian_model.py

+    def from_string_spec(cls, hamiltonian, qubit_list=None):
+        """Initialize from a Hamiltonian string specification.


qubit_list -> subsystems or qargs (they could be oscillator systems etc)

qargs is kwarg uesd for QuantumCircuit and Operator classes for specifying subsystems

Adding this to the list of things for future PRs.

qiskit/providers/aer/openpulse/hamiltonian_model.py

chriseclectic · 2019-12-19T15:51:15Z

qiskit/providers/aer/openpulse/pulse_system_model.py

+
+        # get relevant information from backend
+        defaults = backend.defaults().to_dict()
+        config = backend.configuration().to_dict()


Same for config

qiskit/providers/aer/openpulse/pulse_system_model.py

…eSystemModel._qubit_freq_est

… _oscillator_dims

…kend is in fact a pulse backend

… PulseSystemModel constructor

chriseclectic · 2019-12-20T17:07:07Z

qiskit/providers/aer/backends/pulse_simulator.py

@@ -9,7 +9,7 @@
 # Any modifications or derivative works of this code must retain this
 # copyright notice, and modified files need to carry a notice indicating
 # that they have been altered from the originals.
-# pylint: disable=arguments-differ
+# pylint: disable=arguments-differ, missing-return-type-doc


We shouldn't need to add this pylint skip, what functions were causing the missing-return-type-doc error?

chriseclectic

LGTM, lets do rest of changes in a follow up PR

…ianModel (Qiskit#496) * added PulseSystemModel to extract information about backend relevant to simulation * Moved HamiltonianModel.calculate_frequencies to PulseSystemModel.calculate_channel_frequencies * Modifying constructors for HamiltonianModel to separate object specification from string parsing * added PulseDefaults object into the pulse_simulator, and changed the default memory_slots setting in digest to be equal to the number of qubits * changed digest default 'meas_level' to 2

DanPuzzuoli added 17 commits December 13, 2019 09:54

added sim_system_model to extract information about backend relevant …

f279cf6

…to simulation

refactoring digest to use new sim_system_model

02b0210

writing new functions for testing

5f87a33

fixing minor bug

eb8e412

migrating functions from pulse_sim_utils into HamiltonianModel

260e74b

deleting pulse_sim_utils

8ce56f6

adding docstring to HamiltonianModel.get_qubit_lo_from_drift

163133f

Moved HamiltonianModel.calculate_frequencies to SimSystemModel.calcul…

0ab199e

…ate_channel_frequencies

changed digest_pulse_obj2 to reflect last commit

6f3282f

fixed bugs in calculate_channel_frequencies, and removed import in ut…

38d631d

…ils.__init__ for pulse_sim_utils

Modifying warnings in digest.py, and moving Hamiltonian parsing warni…

9f7dbfd

…ngs to HamiltonianModel

eliminating old functions

c32616a

removing redundant functions that were kept for testing

8399a44

removed import of HamiltonionModel from digest as it is no longer needed

2ee81d6

removing last redundancies

7d8e9bf

fixed bug with how frequencies are being computed in SimSystemModel.c…

f2c76e3

…alculate_channel_frequencies

fixed automatically handling of qubit_list in SimSystemModel

bb1f331

DanPuzzuoli requested review from atilag and chriseclectic as code owners December 13, 2019 19:22

DanPuzzuoli added 6 commits December 13, 2019 14:23

Removed calculate_frequencies from HamiltonianModel, as it is now in …

2acbda0

…SimSystemModel

Modifying constructors for HamiltonianModel to separate object specif…

6f25d4f

…ication from string parsing

changed name of SimSystemModel to SystemModel

22e0589

set backend_options default value to {}, as it is now not a required …

3ec9b52

…field

migrating digest tests to tests for system_model

79c3171

finished migrating tests to test_system_models

72c1473

DanPuzzuoli added 4 commits December 16, 2019 10:24

attempting to change TestPulseSimulator.test_x_gate to new structure

943afe3

Adding functions for channels to system_model

a85462b

Adding functions to SystemModel so channels can also be created from it

6e80bf6

migrating tests to the new style using a temporary new file to keep t…

56eb388

…rack

DanPuzzuoli added 4 commits December 18, 2019 13:04

fixed mistake in example notebook

180e77b

final linting/style corrections

6f997b7

fixing some pylint issues

8ce7774

pylint issues again

1f3c357

DanPuzzuoli changed the title ~~[WIP] Adding SimSystemModel class as higher level class containing HamiltonianModel~~ Adding SimSystemModel class as higher level class containing HamiltonianModel Dec 18, 2019

added PulseDefaults object into the pulse_simulator, and changed the …

6591be9

…default memory_slots setting in digest to be equal to the number of qubits

chriseclectic reviewed Dec 19, 2019

View reviewed changes

DanPuzzuoli added 13 commits December 19, 2019 13:02

changed digest default 'meas_level' to 2

b442fa5

changing from_str_spec to from_dict for HamiltonianModel

7167c1e

attempting to remove HamiltonianModel.set_variables

4e33072

finished removing HamiltonianModel.set_variables from tests

d700275

Changed parameter PulseSystemModel.qubit_freq_est to the private Puls…

7373812

…eSystemModel._qubit_freq_est

removed attribute PulseSystemModel.channels

1440568

in HamiltonianModel, changed _dim_qub and _dim_osc to _qubit_dims and…

65ed47e

… _oscillator_dims

removing .keys() from iteration over dictionary keys

20ab05e

changed oscillator warning to an exception

19a1348

added checking that the backend argument in PulseSystemModel.from_bac…

0bd31e4

…kend is in fact a pulse backend

added checkign that hamiltonian is either None or HamiltonianModel in…

7bd6904

… PulseSystemModel constructor

changed oscillator warning to an exception, and updating examples

8af02fa

fixing style issues

6414455

chriseclectic reviewed Dec 20, 2019

View reviewed changes

chriseclectic approved these changes Dec 20, 2019

View reviewed changes

chriseclectic merged commit 0f2d013 into Qiskit:openpulse-sim Dec 20, 2019

atilag changed the title ~~Adding SimSystemModel class as higher level class containing HamiltonianModel~~ Adding PulseSystemModel class as higher level class containing HamiltonianModel Dec 26, 2019

DanPuzzuoli deleted the pulse-sim-model branch June 4, 2020 13:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding PulseSystemModel class as higher level class containing HamiltonianModel #496

Adding PulseSystemModel class as higher level class containing HamiltonianModel #496

DanPuzzuoli commented Dec 13, 2019 •

edited by atilag

Loading

DanPuzzuoli commented Dec 15, 2019 •

edited

Loading

DanPuzzuoli commented Dec 18, 2019 •

edited

Loading

chriseclectic left a comment

chriseclectic Dec 19, 2019

DanPuzzuoli Dec 19, 2019

chriseclectic Dec 19, 2019

chriseclectic Dec 20, 2019

chriseclectic left a comment

		def from_string_spec(cls, hamiltonian, qubit_list=None):
		"""Initialize from a Hamiltonian string specification.

Adding PulseSystemModel class as higher level class containing HamiltonianModel #496

Adding PulseSystemModel class as higher level class containing HamiltonianModel #496

Conversation

DanPuzzuoli commented Dec 13, 2019 • edited by atilag Loading

Update

Summary

Details and comments

DanPuzzuoli commented Dec 15, 2019 • edited Loading

DanPuzzuoli commented Dec 18, 2019 • edited Loading

chriseclectic left a comment

Choose a reason for hiding this comment

chriseclectic Dec 19, 2019

Choose a reason for hiding this comment

DanPuzzuoli Dec 19, 2019

Choose a reason for hiding this comment

chriseclectic Dec 19, 2019

Choose a reason for hiding this comment

chriseclectic Dec 20, 2019

Choose a reason for hiding this comment

chriseclectic left a comment

Choose a reason for hiding this comment

DanPuzzuoli commented Dec 13, 2019 •

edited by atilag

Loading

DanPuzzuoli commented Dec 15, 2019 •

edited

Loading

DanPuzzuoli commented Dec 18, 2019 •

edited

Loading