Update configurations to perform serialization/deserialization scalings before initializing class by taalexander · Pull Request #4728 · Qiskit/qiskit

taalexander · 2020-07-16T17:05:29Z

Summary

This fixes a bug in the mock backend data. All mock backends were updated as well. Additionally a bug was found in parametric pulse.

Details and comments

taalexander · 2020-07-16T17:08:41Z

I'm not sure how this was working previously?

Apparently this conversion was happening in the provider. It seems like this update will be required for the dumping of files. Its possible I could do this conversion in the mock backend loader @mtreinish ?

I'm not sure I fully follow how this gets used. But yeah we can probably port whatever we need into the fake provider class. We'll probably add a method to the base class to extract the json loading bit and PulseDefaults object creation for all the pulse backends. Right now they just all have lines like:

https://github.com/Qiskit/qiskit-terra/blob/master/qiskit/test/mock/backends/armonk/fake_armonk.py#L55-L63

to do this. If we need to add more common code we should do that in a single place instead of multiple

mtreinish

This LGTM, the only thing I'm not really sure about is changing the types from the __int__ constructor I feel that is potentially a breaking change because in past releases if you did PulseBackendConfiguration(..., dt=x_in_secs)` it would now behave differently than it did in prior releases.

Also, in the past we've tried to make construction from from_dict(input) is roughly equivalent to __init__(**input) (with the exception of nested objects with to_dict/from_dict)so that regardless of how you create the objects the input is roughtly the same.

taalexander · 2020-07-16T19:34:23Z

I'm fine moving it back. My rational was that this decouples the Python representation from the serialized representation. Thumbs up this if you want to switch back.

mtreinish · 2020-07-16T19:49:22Z

I'm fine moving it back. My rational was that this decouples the Python representation from the serialized representation. Thumbs up this if you want to switch back.

Yeah I agree with the idea as a general statement, but I think it's potentially going to break things because it changes the input format for the constructor. So if we want to do this we should come up with a deprecation and migration plan

taalexander · 2020-07-16T20:00:35Z

I'll revert 😄

IBM Quantum has added 3 new backends recently: Bogota, Montreal, and Toronto. This commit creates new fake backends FakeBogota, FakeMontreal, and FakeToronto from snapshots of the real devices.

…rom_dict

…dict

taalexander · 2020-07-16T22:20:55Z

Reverted and rebased on #4733

…ackend-defaults-bug

mtreinish

LGTM, one inline question but probably not a blocker. I was thinking it might be good to add a test for this, probably just adding some asserts to https://github.com/Qiskit/qiskit-terra/blob/master/test/python/providers/test_fake_backends.py#L78 or something to make sure we get these right moving forward.

mtreinish · 2020-07-28T17:57:06Z

+        if self.qubit_lo_range:
+            out_dict['qubit_lo_range'] = [
+                [min_range * 1e-9, max_range * 1e-9] for
+                (min_range, max_range) in self.qubit_lo_range]
+
+        if self.meas_lo_range:
+            out_dict['meas_lo_range'] = [
+                [min_range * 1e-9, max_range * 1e-9] for
+                (min_range, max_range) in self.meas_lo_range]
+
+        if self.rep_times:
+            out_dict['rep_times'] = [_rt * 1e6 for _rt in self.rep_times]


How does this interact with the equivalent lines in QasmBackendConfiguration? I assume we just end up running this conversion twice and overwriting what's in out_dict with this from super().

These are fields are only for pulse backends. They aren't in the QasmBackendConfiguration.

Ignore my previous comment. Apparently they are in the QASM backend. However, they should interact as you said so it shouldn't be an issue.

I guess the follow on question here then is whether we need this duplicated, but it doesn't hurt anything and we can always clean it up in a follow up PR later.

mtreinish

LGTM, thanks for adding some tests. Hopefully this means we won't have issues with the units in the fake backend snapshots moving forward.

mtreinish · 2020-07-30T17:27:19Z

+        if self.qubit_lo_range:
+            out_dict['qubit_lo_range'] = [
+                [min_range * 1e-9, max_range * 1e-9] for
+                (min_range, max_range) in self.qubit_lo_range]
+
+        if self.meas_lo_range:
+            out_dict['meas_lo_range'] = [
+                [min_range * 1e-9, max_range * 1e-9] for
+                (min_range, max_range) in self.meas_lo_range]
+
+        if self.rep_times:
+            out_dict['rep_times'] = [_rt * 1e6 for _rt in self.rep_times]


I guess the follow on question here then is whether we need this duplicated, but it doesn't hurt anything and we can always clean it up in a follow up PR later.

In Qiskit#4728 how we handle the difference between the object units and the serialization format. However the snapshot update didn't actually save the correct values in the json and this is causing issues for tutorials that rely on this. This commit updates the backend snapshots for backends by rerunning the update script to correct this issue.

In #4728 how we handle the difference between the object units and the serialization format. However the snapshot update didn't actually save the correct values in the json and this is causing issues for tutorials that rely on this. This commit updates the backend snapshots for backends by rerunning the update script to correct this issue.

In #4728 how we handle the difference between the object units and the serialization format. However the snapshot update didn't actually save the correct values in the json and this is causing issues for tutorials that rely on this. This commit updates the backend snapshots for backends by rerunning the update script to correct this issue. (cherry picked from commit 0f7673e)

In #4728 how we handle the difference between the object units and the serialization format. However the snapshot update didn't actually save the correct values in the json and this is causing issues for tutorials that rely on this. This commit updates the backend snapshots for backends by rerunning the update script to correct this issue. (cherry picked from commit 0f7673e) Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

In Qiskit#4728 how we handle the difference between the object units and the serialization format. However the snapshot update didn't actually save the correct values in the json and this is causing issues for tutorials that rely on this. This commit updates the backend snapshots for backends by rerunning the update script to correct this issue.

* Update the qobj schema to support pulse gate calibrations. * Fixup qobj classes: add docstrings, call out to super * Move pulse library to top level. Fill in more details in qasm_qobj.py * Fix mistake in schema: the pulse_library definition already contains the 'array' requirement * Move pulse library to top level in qasm_qobj.py as well * Add GateCalibration to qobj/__init__.py * Fixup some implementation errors: gates contains a list of dict items, which needed to be fixed in both the schema and the py files * Put gate calibrations into calibrations.gates to leave room for adding other metadata in the future. Make a qobj.common file to allow qobj.qasm to use features of qobj.pulse * Add an example json * Schema version should be referenced not hardcoded a second time * Pretty print example file * Check if calibrations is present when doing to from dict * Fixup qobj test * Update the qobj schema to support pulse gate calibrations. * Fixup qobj classes: add docstrings, call out to super * Move pulse library to top level. Fill in more details in qasm_qobj.py * Fix mistake in schema: the pulse_library definition already contains the 'array' requirement * Move pulse library to top level in qasm_qobj.py as well * Add GateCalibration to qobj/__init__.py * Fixup some implementation errors: gates contains a list of dict items, which needed to be fixed in both the schema and the py files * Put gate calibrations into calibrations.gates to leave room for adding other metadata in the future. Make a qobj.common file to allow qobj.qasm to use features of qobj.pulse * Add an example json * Schema version should be referenced not hardcoded a second time * Pretty print example file * Update backend snapshots with new conf or defs (#4897) In #4728 how we handle the difference between the object units and the serialization format. However the snapshot update didn't actually save the correct values in the json and this is causing issues for tutorials that rely on this. This commit updates the backend snapshots for backends by rerunning the update script to correct this issue. * One reference to get_sample_pulse is raising deprecation warnings from the assembler (#4903) * Check if calibrations is present when doing to from dict * Fixup qobj test * Add qobj.config.calibrations for common cals * style * Move recent qasm qobj changes to the new qobj.common file * Update schema with qobj level and experiment level calibrations * Add schema test * Apply suggestions from code review Co-authored-by: SooluThomas <soolu.elto@gmail.com> * Fix encoding for validation of new pulse library field in qasm qobj. Add qobj test Co-authored-by: Matthew Treinish <mtreinish@kortar.org> Co-authored-by: SooluThomas <soolu.elto@gmail.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

…gs before initializing class (Qiskit/qiskit#4728) * Add fake backends for Bogota, Montreal, and Toronto IBM Quantum has added 3 new backends recently: Bogota, Montreal, and Toronto. This commit creates new fake backends FakeBogota, FakeMontreal, and FakeToronto from snapshots of the real devices. * Refactor QobjConfiguration transformations to happen in to_dict and from_dict * Refactor PulseDefaults transformations to happen in to_dict and from_dict * Remove conversions from update fake backends scripts. * Update mock backends. Added ibmq_toronto and ibmq_montreal. * fix bug in the parametric pulse code. * Undo from_dict/init changes. * Do conversion of complex number better. * Update mock backend files. * Linting. * Add conversion tests. Co-authored-by: Matthew Treinish <mtreinish@kortar.org> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

taalexander added the bug Something isn't working label Jul 16, 2020

taalexander requested a review from mtreinish July 16, 2020 17:05

taalexander requested review from a team and jyu00 as code owners July 16, 2020 17:05

taalexander commented Jul 16, 2020

View reviewed changes

mtreinish reviewed Jul 16, 2020

View reviewed changes

Add fake backends for Bogota, Montreal, and Toronto

7ee2c5f

IBM Quantum has added 3 new backends recently: Bogota, Montreal, and Toronto. This commit creates new fake backends FakeBogota, FakeMontreal, and FakeToronto from snapshots of the real devices.

mtreinish mentioned this pull request Jul 16, 2020

Add fake backends for Bogota, Montreal, and Toronto #4733

Merged

taalexander added 9 commits July 16, 2020 16:31

Merge branch 'master' into add-new-backends

1d2bfc0

Refactor QobjConfiguration transformations to happen in to_dict and f…

29c1888

…rom_dict

Refactor PulseDefaults transformations to happen in to_dict and from_…

26d97fb

…dict

Remove conversions from update fake backends scripts.

4cd9a30

Update mock backends. Added ibmq_toronto and ibmq_montreal.

eb5273c

fix bug in the parametric pulse code.

305c2ea

Undo from_dict/init changes.

9ce8747

Do conversion of complex number better.

0187fb8

Update mock backend files.

9d7c6e5

taalexander force-pushed the issue-4720-mock-backend-defaults-bug branch from 603d33c to 9d7c6e5 Compare July 16, 2020 22:20

taalexander requested a review from mtreinish July 16, 2020 22:21

Merge remote-tracking branch 'upstream/master' into issue-4720-mock-b…

8358481

…ackend-defaults-bug

taalexander mentioned this pull request Jul 17, 2020

Convert Hamiltonian variables to Hz #3599

Closed

taalexander added 2 commits July 17, 2020 12:28

Merge branch 'master' into issue-4720-mock-backend-defaults-bug

f3430d5

Linting.

c26b56d

mtreinish added this to the 0.15 milestone Jul 28, 2020

mtreinish self-assigned this Jul 28, 2020

kdk linked an issue Jul 28, 2020 that may be closed by this pull request

Mock Backends Default values are not properly being converted back to GHz #4720

Closed

mtreinish reviewed Jul 28, 2020

View reviewed changes

Add conversion tests.

34437f2

mtreinish approved these changes Jul 30, 2020

View reviewed changes

mtreinish added the automerge label Jul 30, 2020

Merge branch 'master' into issue-4720-mock-backend-defaults-bug

994b670

mergify Bot merged commit 3e20d3c into Qiskit:master Jul 30, 2020

mtreinish mentioned this pull request Aug 10, 2020

Update backend snapshots with new conf or defs #4897

Merged

Conversation

taalexander commented Jul 16, 2020

Summary

Details and comments

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtreinish left a comment

Choose a reason for hiding this comment

Uh oh!

taalexander commented Jul 16, 2020

Uh oh!

mtreinish commented Jul 16, 2020

Uh oh!

taalexander commented Jul 16, 2020

Uh oh!

taalexander commented Jul 16, 2020

Uh oh!

mtreinish left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtreinish left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants