Removed code deprecated in 0.22 [transpiler and providers] #10872

Matrixmang0 · 2023-09-20T21:26:56Z

Summary

* Removed the argument 'qubit_channel_mapping' in the class 'qiskit.transpiler.passes.calibration.rzx_builder.RZXCalibrationBuilder'

* Replaced the argument 'qobj[Qobj]' in 'qiskit.providers.aer.QasmSimulator.run()' with 'qcirc[QuantumCircuit]'

* Altered the test files accordingly

* Added a release note

Details and comments

This PR addresses issue #10813. It involves the removal of the qubit_channel_mapping argument from the RZXCalibrationBuilder class and provides a migration example in the release notes. Additionally, this also replaces the qobj argument [Qobj object], has been replaced with qcirc [QuantumCircuit object]. Tests that conflicted with these changes have been modified to accommodate this adjustment.

The test file test_basicaer_qobj_headers.py has been removed, which previously checked for the header component passed into the qobj object and whether it could be retrieved through the result object. Since we are removing the qobj itself, this test can be disregarded, hence it has been deleted, and corresponding migration instructions have been added to the release notes.

This PR has been requested to tagged as Changelog: Removal to reflect the removal deprecated components.

) * Removed the argument 'qubit_channel_mapping' in the class 'qiskit.transpiler.passes.calibration.rzx_builder.RZXCalibrationBuilder' * Replaced the argument 'qobj[Qobj]' in 'qiskit.providers.aer.QasmSimulator.run()' with 'qcirc[QuantumCircuit]' * Altered the test files accordingly * Added a release note

qiskit-bot · 2023-09-20T21:27:00Z

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the the following people are requested to review this:

@Qiskit/terra-core

CLAassistant · 2023-09-20T21:27:01Z

All committers have signed the CLA.

CLAassistant · 2023-09-20T21:27:02Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

ElePT

Thank you very much for your contribution. I took a look at the PR and left a few comments, let me know if anything is unclear :)

ElePT · 2023-09-28T11:34:30Z

releasenotes/notes/remove-deprecated-code-in-0.22.0-139fa09ee0cc6df9.yaml

+    old:
+   	  # Create an RZXCalibrationBuilder object with the qubit_channel_mapping argument 
+      builder = RZXCalibrationBuilder(qubit_channel_mapping=[[0, 1]])
+    new:
+      # Create a target object for the Aer simulator
+      target = providers.aer.QasmSimulator().target()
+      # Create an RZXCalibrationBuilder object using the target object
+      builder = RZXCalibrationBuilder(target=target)


I believe that not specifying these lines as code blocks is confusing Sphinx and breaking the lint and docs build (see the docs workflow). While you can add code blocks with the following syntax:

Here is some reno text... For example: .. code-block:: python # This is a comment # Respecting the indentation of the code block is important x = 1

I think that this example may actually confuse users, as it looks like target is a direct replacement for qubit_channel_mapping, when this not the case. Given that this was a deprecation without replacement, I'd say that we don't need a migration example here. I would directly remove it and keep it simple.

Thank you so much for the feedback. Let me update the .yaml file and push the commit

ElePT · 2023-09-28T11:48:25Z

qiskit/providers/basicaer/qasm_simulator.py

+    def run(self, qcirc, **backend_options):
+        """Run qcirc asynchronously.

        Args:
-            qobj (Qobj): payload of the experiment
+            qcirc (Union[QuantumCircuit, List[QuantumCircuit]]): circuit of the experiment
            backend_options (dict): backend options


From what I have understood, this deprecation was introduced when the interface of BasicAer backends (for example, QasmSimulatorPy) was updated to follow the interface of BackendV1. If you look at the BackendV1 class (link), the run method has a run_input field where you input the circuit, so it would probably make sense to use the name run_input instead of qcirc here.

Thank you so much for pointing it out. I will make the necessary edits and push the commit for review.

ElePT · 2023-09-28T11:55:50Z

releasenotes/notes/remove-deprecated-code-in-0.22.0-139fa09ee0cc6df9.yaml

+    Replaced the argument `qobj[Qobj]` in :meth:`qiskit.providers.aer.QasmSimulator.run()` with `qcirc[QuantumCircuit]`
+
+    old:
+      # Transpile the circuit for the target simulator
+      simulator = QasmSimulator()
+      transpiled_circuit = transpile(qc, simulator)
+      # Assemble the transpiled circuit into a Qobj
+      qobj = assemble(transpiled_circuit, shots=1024)
+      # Run the simulation
+      job = simulator.run(qobj)
+    new:
+      # Transpile the circuit for the target simulator
+      simulator = QasmSimulator()
+      transpiled_circuit = transpile(qc, simulator)
+      # Run the simulation
+      job = simulator.run(transpiled_circuit, shots=1024)


I would also keep this second section very simple, the deprecation was already explained in the original PR (see reno here). So here, I would get rid of the "old" section and just mention that the deprecated qobj input has been removed and an example with the new use (in a code block as explained above).

Definitely will do and thank you so much for your continuous feedback. Being new to the OS contributions, all your suggestions were invaluable for me.

…ove-deprecate

ElePT

Thanks a lot! I have added a small comment, but the changes look good to me. Because I am not that familiar with the original deprecation. I'd rather have a second reviewer check that this is the intended change (@1ucian0), especially with the qobj -> run_input rename.

ElePT · 2023-09-29T08:27:10Z

qiskit/providers/basicaer/qasm_simulator.py

+                For legacy providers migrating to the new versioned providers,
+                provider interface a :class:`~qiskit.qobj.QasmQobj` or
+                :class:`~qiskit.qobj.PulseQobj` objects should probably be
+                supported too (but deprecated) for backwards compatibility. Be
+                sure to update the docstrings of subclasses implementing this
+                method to document that. New provider implementations should not
+                do this though as :mod:`qiskit.qobj` will be deprecated and
+                removed along with the legacy providers interface.


This part of the docstring doesn't look necessary here.

Thank you once more for your assistance. I too felt the same, let me retain the original wording in the string. Your help is greatly appreciated.

…ove-deprecate

into remove-deprecate

1ucian0

LGTM. A very impressive first contribution.

since the qubit_channel_mapping deprecation was introduced by @nkanazawa1989 in #8276, maybe better if he also have a look.

Similarly, #5128 by @mtreinish deprecatd the qobj support in QasmSimulatorPy. Is the removal as inteneded?

Thanks!

qiskit/providers/basicaer/qasm_simulator.py

releasenotes/notes/remove-deprecated-code-in-0.22.0-139fa09ee0cc6df9.yaml

Formatted the docstring of .run() fnction as per the review suggestion Co-authored-by: Luciano Bello <[email protected]>

…cc6df9.yaml Added extra detail on the version of ``qiskit-terra`` from which ``qubit_channel_mapping`` was depreciated Co-authored-by: Luciano Bello <[email protected]>

…cc6df9.yaml Better formatted the release notes Co-authored-by: Luciano Bello <[email protected]>

Matrixmang0 · 2023-10-03T17:31:24Z

LGTM. A very impressive first contribution.

since the qubit_channel_mapping deprecation was introduced by @nkanazawa1989 in #8276, maybe better if he also have a look.

Similarly, #5128 by @mtreinish deprecatd the qobj support in QasmSimulatorPy. Is the removal as inteneded?

Thanks!

Thank you for your kind words; they mean a lot. That was great encouragement for me. Special thanks to @ElePT for their guidance—it made this contribution possible. Excited to contribute more and be part of the community!

coveralls · 2023-10-03T17:54:39Z

Pull Request Test Coverage Report for Build 6396275891

8 of 9 (88.89%) changed or added relevant lines in 1 file are covered.
13 unchanged lines in 3 files lost coverage.
Overall coverage decreased (-0.01%) to 87.016%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/providers/basicaer/qasm_simulator.py	8	9	88.89%

Files with Coverage Reduction	New Missed Lines	%
qiskit/providers/basicaer/unitary_simulator.py	1	80.26%
crates/qasm2/src/lex.rs	6	90.91%
crates/qasm2/src/parse.rs	6	97.6%

Totals
Change from base Build 6394675399:	-0.01%
Covered Lines:	74153
Relevant Lines:	85218

💛 - Coveralls

mtreinish

This LGTM, thanks for taking care of this. I just caught one small mistake in the updated docstring for basicaer that I commented on inline.

I also just realized that qiskit.providers.basicaer.UnitarySimulatorPy.run() (https://github.com/Qiskit/qiskit/blob/main/qiskit/providers/basicaer/unitary_simulator.py#L206) never deprecated a Qobj input and will still support it while the qasm simulator and statevector simulator will not. I still think we should move forward with your PR, but it's something we'll want to clean up in a subsequent PR.

qiskit/providers/basicaer/qasm_simulator.py

1ucian0 · 2023-10-19T11:48:39Z

I still think we should move forward with your PR, but it's something we'll want to clean up in a subsequent PR.

#11046

Co-authored-by: Matthew Treinish <[email protected]>

Matrixmang0 requested review from jyu00 and a team as code owners September 20, 2023 21:26

qiskit-bot added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Sep 20, 2023

Matrixmang0 changed the title ~~Removed code deprecated in 0.22 [transpiler and providers] (#10813)~~ Removed code deprecated in 0.22 [transpiler and providers] Sep 21, 2023

1ucian0 added the Changelog: Removal Include in the Removed section of the changelog label Sep 21, 2023

1ucian0 self-assigned this Sep 22, 2023

Matrixmang0 force-pushed the remove-deprecate branch from d162af9 to 2e5eee5 Compare September 24, 2023 13:21

ElePT reviewed Sep 28, 2023

View reviewed changes

Matrixmang0 added 2 commits September 28, 2023 22:52

Merge branch 'main' of https://github.com/Matrixmang0/qiskit into rem…

13398e9

…ove-deprecate

Update release note and change input argument variable name

12f5a50

Matrixmang0 force-pushed the remove-deprecate branch from 4ceb9e6 to 12f5a50 Compare September 28, 2023 18:06

Merge branch 'main' into remove-deprecate

98c1a28

Matrixmang0 requested a review from ElePT September 28, 2023 21:06

ElePT reviewed Sep 29, 2023

View reviewed changes

Matrixmang0 added 4 commits September 30, 2023 10:31

Merge branch 'Qiskit:main' into remove-deprecate

16bf6d5

Merge branch 'main' of https://github.com/Matrixmang0/qiskit into rem…

f99f7cc

…ove-deprecate

Merge branch 'remove-deprecate' of https://github.com/Matrixmang0/qiskit

143f1eb

into remove-deprecate

Edited the docstring for the arg run_input

3bb4c80

1ucian0 reviewed Oct 3, 2023

View reviewed changes

Matrixmang0 and others added 3 commits October 3, 2023 22:42

Update qiskit/providers/basicaer/qasm_simulator.py

a978fc6

Formatted the docstring of .run() fnction as per the review suggestion Co-authored-by: Luciano Bello <[email protected]>

Update releasenotes/notes/remove-deprecated-code-in-0.22.0-139fa09ee0…

8b36424

…cc6df9.yaml Added extra detail on the version of ``qiskit-terra`` from which ``qubit_channel_mapping`` was depreciated Co-authored-by: Luciano Bello <[email protected]>

Update releasenotes/notes/remove-deprecated-code-in-0.22.0-139fa09ee0…

88c4419

…cc6df9.yaml Better formatted the release notes Co-authored-by: Luciano Bello <[email protected]>

1ucian0 assigned mtreinish and nkanazawa1989 Oct 3, 2023

Merge branch 'main' into remove-deprecate

72b0544

Merge branch 'main' into remove-deprecate

0dc1ce5

1ucian0 added this to the 0.45.0 milestone Oct 18, 2023

mtreinish reviewed Oct 18, 2023

View reviewed changes

qiskit/providers/basicaer/qasm_simulator.py Outdated Show resolved Hide resolved

1ucian0 mentioned this pull request Oct 19, 2023

Deprecate Qobj in unitary_simulator.py #11046

Closed

Update qiskit/providers/basicaer/qasm_simulator.py

1bbbb3b

Co-authored-by: Matthew Treinish <[email protected]>

1ucian0 approved these changes Oct 19, 2023

View reviewed changes

1ucian0 enabled auto-merge October 19, 2023 11:50

1ucian0 added this pull request to the merge queue Oct 19, 2023

Merged via the queue into Qiskit:main with commit eef868d Oct 19, 2023
13 checks passed

Matrixmang0 deleted the remove-deprecate branch October 19, 2023 15:31

Matrixmang0 restored the remove-deprecate branch October 19, 2023 15:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Removed code deprecated in 0.22 [transpiler and providers] #10872

Removed code deprecated in 0.22 [transpiler and providers] #10872

Matrixmang0 commented Sep 20, 2023 •

edited

Loading

qiskit-bot commented Sep 20, 2023

CLAassistant commented Sep 20, 2023 •

edited

Loading

CLAassistant commented Sep 20, 2023

ElePT left a comment

ElePT Sep 28, 2023

Matrixmang0 Sep 28, 2023

ElePT Sep 28, 2023

Matrixmang0 Sep 28, 2023

ElePT Sep 28, 2023

Matrixmang0 Sep 28, 2023

ElePT left a comment

ElePT Sep 29, 2023

Matrixmang0 Sep 30, 2023

1ucian0 left a comment

Matrixmang0 commented Oct 3, 2023

coveralls commented Oct 3, 2023

mtreinish left a comment

1ucian0 commented Oct 19, 2023

Removed code deprecated in 0.22 [transpiler and providers] #10872

Removed code deprecated in 0.22 [transpiler and providers] #10872

Conversation

Matrixmang0 commented Sep 20, 2023 • edited Loading

Summary

Details and comments

qiskit-bot commented Sep 20, 2023

CLAassistant commented Sep 20, 2023 • edited Loading

CLAassistant commented Sep 20, 2023

ElePT left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ElePT left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ucian0 left a comment

Choose a reason for hiding this comment

Matrixmang0 commented Oct 3, 2023

coveralls commented Oct 3, 2023

Pull Request Test Coverage Report for Build 6396275891

💛 - Coveralls

mtreinish left a comment

Choose a reason for hiding this comment

1ucian0 commented Oct 19, 2023

Matrixmang0 commented Sep 20, 2023 •

edited

Loading

CLAassistant commented Sep 20, 2023 •

edited

Loading