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

ConfigurableBackendV2 for easily creating mock backendv2 instances #7501

Closed
wants to merge 8 commits into from
Closed

ConfigurableBackendV2 for easily creating mock backendv2 instances #7501

wants to merge 8 commits into from

Conversation

evmckinney9
Copy link
Contributor

@evmckinney9 evmckinney9 commented Jan 7, 2022

Summary

Addresses #7499.

Details and comments

This initial commit contains some remaining TODOs.
I used FakeMumbaiV2 and ConfigurableBackend(V1) to create this. I tried to make this simple so you don't have to pass a ton to this factory. The main parameter is in a dictionary that contains the gates and the qubits they can be applied on. The rest is just dictionary comprehension to build the InstructionProperties and QubitProperties.

@evmckinney9 evmckinney9 requested a review from a team as a code owner January 7, 2022 22:05
@evmckinney9
Copy link
Contributor Author

evmckinney9 commented Jan 7, 2022

Example usage:

class FakeExampleV2(ConfigurableFakeBackendV2):
    """A mock backendv2"""

    def __init__(self):
        qubits = list(range(4))
        coupling_map = [[0, 1], [0, 2], [0, 3], [1, 2]]
        qubit_coordinates = [[0, 1], [1, 0], [1, 1], [1, 2]]

        gate_configuration = {}
        gate_configuration[IGate] = [(i,) for i in qubits]

        # only can do RXGates on qubits 0 and 4
        gate_configuration[RXGate] = [
            (i,) for i in list(set(qubits).difference([1, 2]))
        ]
        # can do RY on all qubits
        gate_configuration[RYGate] = [(i,) for i in qubits]

        # can do CZ on all pairs in coupling map
        gate_configuration[CZGate] = [(i, j) for i, j in coupling_map]

        # can only measure qubits 2,3
        measurable_qubits = [2, 3]

        super().__init__(
            name="mock_example",
            description="a mock backend",
            n_qubits=len(qubits),
            gate_configuration=gate_configuration,
            parameterized_gates={RXGate: "theta", RYGate: "theta"},
            measurable_qubits=measurable_qubits,
            qubit_coordinates=qubit_coordinates,
        )

@HuangJunye
Copy link
Collaborator

@evmckinney9 Thanks for your valuable contributions! I am sorry for taking so long to give you a response.

@IceKhan13 I took a look through the history of ConfigurableFakeBackend #3797. Can you please help review this PR? I am starting to read about BackendV2 so if you are unsure about the changes of V2 you can ask me. Thank you!

@coveralls
Copy link

Pull Request Test Coverage Report for Build 1773260406

  • 22 of 68 (32.35%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.05%) to 83.102%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/test/mock/utils/configurable_backend_v2.py 21 67 31.34%
Totals Coverage Status
Change from base Build 1763987014: -0.05%
Covered Lines: 51932
Relevant Lines: 62492

💛 - Coveralls

Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

Thanks for starting this, it's good to see BackendV2 used here. I left some questions and comments inline about some changes we should make. Besides those can you add a new features release note about this new class (the details on how to do this are documented here: https://github.com/Qiskit/qiskit-terra/blob/main/CONTRIBUTING.md#release-notes ). Also it would be really great if there were some unit tests that were leveraging this class. I realize that until there is run() support in the class we can't do a full path execution test on aer, but at least validating the construction of ConfigurableFakeBackendV2 objects and that we can use them with transpile() would be very useful in this PR.

return Options(shots=1024)

def run(self, run_input, **options):
raise NotImplementedError
Copy link
Member

Choose a reason for hiding this comment

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

I think here we probably should have a comment TODO: Remove after #7391 is implemented and this is based on FakeBackendV2. Since I think this class being able to used as a simulation target is useful.

Comment on lines +72 to +99
if std is None:
std = 0.01

if measurable_qubits is None:
measurable_qubits = list(range(n_qubits))

if not isinstance(qubit_t1, list):
qubit_t1 = np.random.normal(
loc=qubit_t1 or 113.0, scale=std, size=n_qubits
).tolist()

if not isinstance(qubit_t2, list):
qubit_t2 = np.random.normal(
loc=qubit_t1 or 150.2, scale=std, size=n_qubits
).tolist()

if not isinstance(qubit_frequency, list):
qubit_frequency = np.random.normal(
loc=qubit_frequency or 4.8, scale=std, size=n_qubits
).tolist()

if not isinstance(qubit_readout_error, list):
qubit_readout_error = np.random.normal(
loc=qubit_readout_error or 0.04, scale=std, size=n_qubits
).tolist()

if dt is None:
dt = 0.2222222222222222e-9
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if there is a way we can de-duplicate this with ConfigurableBackendV1 as this is identical to the previous implementation. It would be easier to maintain if we need to change something if there was only one place to update.

self,
name: str,
description: str,
n_qubits: int,
Copy link
Member

Choose a reason for hiding this comment

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

We've standardized on using num_qubits everywhere in the qiskit api (the only exception was the models in qiskit.providers.models as those were documented externally as part of the payload specification those are based on). So it would be better to be consistent here and use num_qubits instead.

Comment on lines +119 to +120
online_date=datetime.datetime.utcnow(),
backend_version="0.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

These two attributes aren't strictly needed as they're optional in the BackendV2 class. Is there an advantage to populating them, or can we just not set it?

self.qubit_readout_error = qubit_readout_error
self.single_qubit_gates = single_qubit_gates
self.std = std
# self.n_qubits = n_qubits
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# self.n_qubits = n_qubits

}
if gate in parameterized_gates.keys():
self._target.add_instruction(
gate(Parameter(parameterized_gates[gate])), temp_gate_props
Copy link
Member

Choose a reason for hiding this comment

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

What about gates with more than 1 parameter. For example, if I want to use a U which has 3 parameters I don't think this would work because I can't specify the 3 different parameter names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In instantiation, pass paramerized gates dictionary with values as a list

parameterized_gates={U3Gate: ["theta", "phi", "lam"], RYGate: ["theta"]},
Suggested change
gate(Parameter(parameterized_gates[gate])), temp_gate_props
gate(*[Parameter(p) for p in parameterized_gates[gate]]),
temp_gate_props,

# TODO: dynamic instruction properties
for gate, qubit_tuple_list in self.gate_configuration.items():
temp_gate_props = {
qubit_tuple: InstructionProperties(duration=0.0, error=0)
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be better to use:

Suggested change
qubit_tuple: InstructionProperties(duration=0.0, error=0)
qubit_tuple: None

if you're not going to set a duration or error rate. Looking at what the v1 version of this class did they set error to 0.01 and 4*dt for the duration. We could just use that here to start.

qubit_t2: Optional[Union[float, List[float]]] = None,
qubit_frequency: Optional[Union[float, List[float]]] = None,
qubit_readout_error: Optional[Union[float, List[float]]] = None,
single_qubit_gates: Optional[List[str]] = None,
Copy link
Member

Choose a reason for hiding this comment

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

How is this parameter used? It doesn't seem to match up with gate_configuration or parameterized_gates. For example, you could specify completely different gates here and there would be nothing that errors. I'm just wondering if we can remove this argument?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like this was accidentally left in from an old version - can be removed.

gate_configuration: Dict[Gate, List[Tuple[int]]],
measurable_qubits: List[int] = None,
parameterized_gates: Optional[Dict[Gate, str]] = None,
qubit_coordinates: Optional[List[List[int]]] = None,
Copy link
Member

Choose a reason for hiding this comment

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

How is this parameter used? I don't see it on V1 and am not familiar with a similar parameter on other backends. Is this just specifying a layout for visualization (which is how I interpret the docstring below)? If so where does this get used because I've not seen this parameter anywhere before.

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 was using qubit_coodinates to specify visualization layouts, see #7494

@HuangJunye HuangJunye mentioned this pull request Mar 2, 2022
9 tasks
@HuangJunye
Copy link
Collaborator

@evmckinney9 I have been working on PR #7643 that are closely related to this one. Would you like to update your PR to leverage on the changes in #7643 after it's merged (hopefully in a week). We can discuss together on the changes.

@HuangJunye HuangJunye added the mod: fake_provider Related to the fake_provider module and fake backends label Apr 1, 2022
@HuangJunye HuangJunye added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Jun 21, 2022
@javabster
Copy link
Contributor

hi @evmckinney9 are you still working on this?

@evmckinney9
Copy link
Contributor Author

I have not maintained it since writing this PR

@javabster
Copy link
Contributor

Ok thanks for letting us know @evmckinney9, I'll close the PR but if you want to return at a later date just let us know and we can reopen it 😄

@javabster javabster closed this Apr 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community PR PRs from contributors that are not 'members' of the Qiskit repo mod: fake_provider Related to the fake_provider module and fake backends
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants