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

Fix handling of circuit metadata #1436

Merged
merged 14 commits into from
Mar 29, 2022

Conversation

hhorii
Copy link
Collaborator

@hhorii hhorii commented Jan 25, 2022

Summary

#1435

Details and comments

to_json is called for each circuit header.
A header may include metadata and metadata can be a python object.
This PR changes serialization of circuit headers to use py::handle without serializing to a json.

Another solution can be in _assemble() to remove all unnecessary metadata from input.

@hhorii hhorii force-pushed the parse_metadata_without_to_json branch 4 times, most recently from 06d98a3 to efde95e Compare January 25, 2022 04:42
@jakelishman
Copy link
Member

I'm not very knowledgeable about the C++/Python handoff with pybind11, so a slight question: what we need out of the header seems very simple - can we just leave the metadata in Python space, and reattach it to the new output circuit (if there is one)? It feels like we could save time by just not serialising it, if we don't use it.

@hhorii
Copy link
Collaborator Author

hhorii commented Jan 26, 2022

what we need out of the header seems very simple - can we just leave the metadata in Python space, and reattach it to the new output circuit (if there is one)?

I believe that py::handle can be passed through C++ space. I will try it.
The current implementation of ExperimentResult uses json_t. To pass py::handle instead of json_t, as other codes, we need to make [ExperimentResult] class a template class to handle both of json_t and py::handle, and modify to_python(ExperimentResult) to handle the both types.

@hhorii hhorii force-pushed the parse_metadata_without_to_json branch from efde95e to cb6e908 Compare February 26, 2022 02:12
@hhorii hhorii changed the title [WIP] use python parser for circuit.header use python parser for circuit.header Feb 26, 2022
@hhorii hhorii force-pushed the parse_metadata_without_to_json branch from cb6e908 to 81992f6 Compare February 26, 2022 03:10
@kevinsung
Copy link

Please add a test that ensures the repro code from #1435 works.

@hhorii hhorii force-pushed the parse_metadata_without_to_json branch from 16e7818 to b075ec0 Compare February 28, 2022 02:39
@hhorii
Copy link
Collaborator Author

hhorii commented Feb 28, 2022

Please add a test that ensures the repro code from #1435 works.

I added test cases to check metadata is copied correctly.

@hhorii hhorii force-pushed the parse_metadata_without_to_json branch from 667d00e to f47fd62 Compare February 28, 2022 04:14
@hhorii hhorii force-pushed the parse_metadata_without_to_json branch from f47fd62 to 562e8cb Compare February 28, 2022 04:30
@hhorii hhorii added the stable-backport-potential The issue or PR might be minimal and/or import enough to backport to stable label Feb 28, 2022
@hhorii
Copy link
Collaborator Author

hhorii commented Mar 1, 2022

TODO:

  • Use pop instead of del
  • Store an index of original circuit for parameterized circuit.
  • Change _assemble instead of _run
  • Do not change codes to copy headers in controller

@hhorii
Copy link
Collaborator Author

hhorii commented Mar 7, 2022

Aer does not use Metadata to simulate circuits. Therefore, I believe, we can reduce metadata from qobj before simulation.

Previously, metadata of QuantumCircuit is copied to QobjExperimentHeader in a qobj. ThisQobjExperimentHeader is returned as header of ExperimentResult. Whit this change, header of ExperimentResult does not keep metadata of QuantumCircuit because it is removed before generating qobj.

qiskit/providers/aer/backends/aerbackend.py Outdated Show resolved Hide resolved
qiskit/providers/aer/backends/aerbackend.py Outdated Show resolved Hide resolved
kevinsung
kevinsung previously approved these changes Mar 7, 2022
Copy link

@kevinsung kevinsung 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 or someone else with write access please take a look.

@hhorii hhorii mentioned this pull request Mar 14, 2022
1 task
@hhorii hhorii requested a review from mtreinish March 15, 2022 14:18
@hhorii hhorii force-pushed the parse_metadata_without_to_json branch 2 times, most recently from 4396035 to c7ed644 Compare March 17, 2022 08:59
@hhorii hhorii force-pushed the parse_metadata_without_to_json branch from c7ed644 to fd65ac7 Compare March 17, 2022 09:28
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.

Overall LGTM, just one question inline and a suggestion on the release note. Thanks for updating this

@mtreinish mtreinish changed the title use python parser for circuit.header Fix handling of circuit metadata Mar 18, 2022
@mtreinish mtreinish added the Changelog: Bugfix Include in the Fixed section of the changelog label Mar 18, 2022
@hhorii
Copy link
Collaborator Author

hhorii commented Mar 29, 2022

I realized that id in metadata is necessary for clustering. QObj is divided to sub-QObjs (first level) and each sub-QObj can be divided to subsub-QObj (second level). If id is removed in the first level, there is no way to merge results in second level.

@hhorii hhorii force-pushed the parse_metadata_without_to_json branch from 976deb7 to 9392f75 Compare March 29, 2022 11:34
@hhorii hhorii added this to the Aer 0.10.4 milestone Mar 29, 2022
@mtreinish mtreinish merged commit 73b29ad into Qiskit:main Mar 29, 2022
hhorii added a commit to hhorii/qiskit-aer that referenced this pull request Mar 30, 2022
The to_json() method  is called for each circuit header.
A header may include metadata and metadata can be a python object.
This PR changes serialization of circuit headers to use py::handle without serializing to a json.

Fixes Qiskit#1435


* use python parser for circuit.header

* support metadata copy with parameterization

* avoid serialization of circuit metadata

* use circuit_index to specify metadata

* remove metadata from qobj for Aer to simulate circuits

* add release note

* clear circuite metadata correctly.

* take unnecessary tests for circuit metadata backup/recovery

* work around metadata serialization issue within _run method

* Update releasenotes/notes/remove_circuit_metadata_from_qobj-324e7ea9b369ee67.yaml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the Fixed section of the changelog stable-backport-potential The issue or PR might be minimal and/or import enough to backport to stable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants