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

[WIP] Add M3 measurement mitigation #6748

Closed
wants to merge 51 commits into from
Closed

[WIP] Add M3 measurement mitigation #6748

wants to merge 51 commits into from

Conversation

nonhermitian
Copy link
Contributor

@nonhermitian nonhermitian commented Jul 14, 2021

Summary

Adds M3 measurement mitigation to Qiskit (currently just the uncorrelated case).

Things to do:

  • Add tests after converting from pytest.
  • Build on top of a generic mitigator base class.
  • Rename grabbing of cal data to something more generic.
  • Update the Quasiprobs class to handle mitigation overhead.
  • Remove internal Quasiprobs objects from code.

Details and comments

@nonhermitian nonhermitian added the Changelog: New Feature Include in the "Added" section of the changelog label Jul 14, 2021
@nonhermitian nonhermitian self-assigned this Jul 14, 2021
@nonhermitian nonhermitian requested a review from a team as a code owner July 14, 2021 14:37
@nonhermitian
Copy link
Contributor Author

This is finally passing.

@mtreinish I am not sure how to inject the tmp dirs the orjson tests need in CI, so I just skip the tests for now.

There was also a bunch of changes to the distribution classes, namely the keys were reporting as integers (done here #6554). This is problematic as things are nominally bitstrings, and there is a limit of 63 qubits above which things like casting to int64 in lower level code fails. I also thinks that that prevents viewing counts and mitigated probabilities together in plot_histogram. I removed all that code to get the tests to pass, although things like casting hex to bitstrings within the distributiion objects themselves, as opposed to the Result seems to be the way to go.

@nonhermitian nonhermitian mentioned this pull request Jul 16, 2021
5 tasks
@nonhermitian nonhermitian added the on hold Can not fix yet label Jul 16, 2021
nonhermitian and others added 3 commits July 16, 2021 09:37
This commit migrates away from using orjson to stdlib json. While orjson
is a great package providing much faster json serialization and native
numpy support (along with other types the stdlib doesn't support out of
the box) adding it as another dependency to qiskit is pretty heavy
especially since terra's json usage is very minimal (besides this PR
mainly testing and metadata serialization for qpy). If we were using
json serialization or deserialization for a core function of terra it
would make more sense as a requirement. We can do the same thing with
stdlib that orjson was providing (although slower) and not need the
new dependency. As part of this the tests for generating and loading
the json cal data are updated so they work with stdlib unit test
fixtures instead of pytest's custom fixtures.
@mtreinish
Copy link
Member

@nonhermitian I've updated this to fix the tests to be compatible with stdlib unittest and also updated the json usage to use stdlib json instead of orjson.

mtreinish added a commit to mtreinish/qiskit-core that referenced this pull request Aug 4, 2021
This commit migrates the measurement mitigation code from qiskit-ignis
into qiskit-terra for use with the QuantumInstance class. The
QuantumInstance class's usage of the measurement mitigation from ignis
is the one thing blocking us from deprecating qiskit-ignis completely.
By embedding the code the quantum instance depends on inside
qiskit.utils users of QuantumInstance (and therefore qiskit.algorithms)
can construct and use measurement mitigation in it's current form. The
use of this migrated module is only supported for use with the
QuantumInstance class and is explicitly documented as internal/private
except for how it gets used by the QuantumInstance.

There is ongoing work to create a standardized mitigation API in Qiskit#6485
and Qiskit#6748, this does not preclude that work, but we should adapt this as
part of those efforts to use the standardized interface. Ideally this
would have been made a private interface and not exposed it as user facing
(in deference to the standardization effort), but unfortunately the
QuantumInstance expects classes of these classes as it's public interface
for selecting a mitigation technique which means users need to be able to
use the classes. However, as only the classes are public interfaces we
can adapt this as we come up with a standardized mitigation interface
and rewrite the internals of this and how the QuantumInstance leverages
mitigators to use the new interface.

A good follow-up here would be to adapt the mitigator selection kwarg to
deprecate the use of classes and then we can make things explicitly
private in the migrated code and wait for Qiskit#6495 and Qiskit#6748 to be ready
for our user facing API. I opted to not include that in this PR to
minimize changes to just what we migrated from ignis and update usage of
old ignis classes to rely on the migrated version.
@nonhermitian
Copy link
Contributor Author

Given that the Qiskit Partners repo is now available for all projects that meet the requirements, and the desire to have some place to point to when the corresponding M3 paper goes out, I am closing this PR and will just make a new repo in Partners. This will also allow for the inclusion of GPU implementations that are outside the scope of Qiskit proper.

@nonhermitian nonhermitian deleted the add_m3 branch August 16, 2021 11:01
kdk pushed a commit that referenced this pull request Oct 1, 2021
* Migrate measure mitigation from ignis for QuantumInstance

This commit migrates the measurement mitigation code from qiskit-ignis
into qiskit-terra for use with the QuantumInstance class. The
QuantumInstance class's usage of the measurement mitigation from ignis
is the one thing blocking us from deprecating qiskit-ignis completely.
By embedding the code the quantum instance depends on inside
qiskit.utils users of QuantumInstance (and therefore qiskit.algorithms)
can construct and use measurement mitigation in it's current form. The
use of this migrated module is only supported for use with the
QuantumInstance class and is explicitly documented as internal/private
except for how it gets used by the QuantumInstance.

There is ongoing work to create a standardized mitigation API in #6485
and #6748, this does not preclude that work, but we should adapt this as
part of those efforts to use the standardized interface. Ideally this
would have been made a private interface and not exposed it as user facing
(in deference to the standardization effort), but unfortunately the
QuantumInstance expects classes of these classes as it's public interface
for selecting a mitigation technique which means users need to be able to
use the classes. However, as only the classes are public interfaces we
can adapt this as we come up with a standardized mitigation interface
and rewrite the internals of this and how the QuantumInstance leverages
mitigators to use the new interface.

A good follow-up here would be to adapt the mitigator selection kwarg to
deprecate the use of classes and then we can make things explicitly
private in the migrated code and wait for #6495 and #6748 to be ready
for our user facing API. I opted to not include that in this PR to
minimize changes to just what we migrated from ignis and update usage of
old ignis classes to rely on the migrated version.

* Update releasenotes/notes/ignis-mitigators-70492690cbcf99ca.yaml

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>

* Finish release notes

* Move API stability warning to the top

* Apply suggestions from code review

Co-authored-by: Jake Lishman <jake@binhbar.com>

* Remove unecessary test subclassing

* Fix skip logic in algorithm mitigation tests

* Fix exception handling on missing ignis in algorithms

* Assert deprecation message for ignis use mentions ignis

* Make filters module properly private

* Fix lint

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Jake Lishman <jake@binhbar.com>
ElePT pushed a commit to ElePT/qiskit that referenced this pull request Jun 27, 2023
* Migrate measure mitigation from ignis for QuantumInstance

This commit migrates the measurement mitigation code from qiskit-ignis
into qiskit-terra for use with the QuantumInstance class. The
QuantumInstance class's usage of the measurement mitigation from ignis
is the one thing blocking us from deprecating qiskit-ignis completely.
By embedding the code the quantum instance depends on inside
qiskit.utils users of QuantumInstance (and therefore qiskit.algorithms)
can construct and use measurement mitigation in it's current form. The
use of this migrated module is only supported for use with the
QuantumInstance class and is explicitly documented as internal/private
except for how it gets used by the QuantumInstance.

There is ongoing work to create a standardized mitigation API in Qiskit#6485
and Qiskit#6748, this does not preclude that work, but we should adapt this as
part of those efforts to use the standardized interface. Ideally this
would have been made a private interface and not exposed it as user facing
(in deference to the standardization effort), but unfortunately the
QuantumInstance expects classes of these classes as it's public interface
for selecting a mitigation technique which means users need to be able to
use the classes. However, as only the classes are public interfaces we
can adapt this as we come up with a standardized mitigation interface
and rewrite the internals of this and how the QuantumInstance leverages
mitigators to use the new interface.

A good follow-up here would be to adapt the mitigator selection kwarg to
deprecate the use of classes and then we can make things explicitly
private in the migrated code and wait for Qiskit#6495 and Qiskit#6748 to be ready
for our user facing API. I opted to not include that in this PR to
minimize changes to just what we migrated from ignis and update usage of
old ignis classes to rely on the migrated version.

* Update releasenotes/notes/ignis-mitigators-70492690cbcf99ca.yaml

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>

* Finish release notes

* Move API stability warning to the top

* Apply suggestions from code review

Co-authored-by: Jake Lishman <jake@binhbar.com>

* Remove unecessary test subclassing

* Fix skip logic in algorithm mitigation tests

* Fix exception handling on missing ignis in algorithms

* Assert deprecation message for ignis use mentions ignis

* Make filters module properly private

* Fix lint

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Jake Lishman <jake@binhbar.com>
ElePT pushed a commit to ElePT/qiskit-algorithms-test that referenced this pull request Jul 17, 2023
…kit#6867)

* Migrate measure mitigation from ignis for QuantumInstance

This commit migrates the measurement mitigation code from qiskit-ignis
into qiskit-terra for use with the QuantumInstance class. The
QuantumInstance class's usage of the measurement mitigation from ignis
is the one thing blocking us from deprecating qiskit-ignis completely.
By embedding the code the quantum instance depends on inside
qiskit.utils users of QuantumInstance (and therefore qiskit.algorithms)
can construct and use measurement mitigation in it's current form. The
use of this migrated module is only supported for use with the
QuantumInstance class and is explicitly documented as internal/private
except for how it gets used by the QuantumInstance.

There is ongoing work to create a standardized mitigation API in Qiskit/qiskit#6485
and Qiskit/qiskit#6748, this does not preclude that work, but we should adapt this as
part of those efforts to use the standardized interface. Ideally this
would have been made a private interface and not exposed it as user facing
(in deference to the standardization effort), but unfortunately the
QuantumInstance expects classes of these classes as it's public interface
for selecting a mitigation technique which means users need to be able to
use the classes. However, as only the classes are public interfaces we
can adapt this as we come up with a standardized mitigation interface
and rewrite the internals of this and how the QuantumInstance leverages
mitigators to use the new interface.

A good follow-up here would be to adapt the mitigator selection kwarg to
deprecate the use of classes and then we can make things explicitly
private in the migrated code and wait for Qiskit/qiskit#6495 and Qiskit/qiskit#6748 to be ready
for our user facing API. I opted to not include that in this PR to
minimize changes to just what we migrated from ignis and update usage of
old ignis classes to rely on the migrated version.

* Update releasenotes/notes/ignis-mitigators-70492690cbcf99ca.yaml

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>

* Finish release notes

* Move API stability warning to the top

* Apply suggestions from code review

Co-authored-by: Jake Lishman <jake@binhbar.com>

* Remove unecessary test subclassing

* Fix skip logic in algorithm mitigation tests

* Fix exception handling on missing ignis in algorithms

* Assert deprecation message for ignis use mentions ignis

* Make filters module properly private

* Fix lint

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Jake Lishman <jake@binhbar.com>
ElePT pushed a commit to ElePT/qiskit-algorithms that referenced this pull request Jul 27, 2023
…kit#6867)

* Migrate measure mitigation from ignis for QuantumInstance

This commit migrates the measurement mitigation code from qiskit-ignis
into qiskit-terra for use with the QuantumInstance class. The
QuantumInstance class's usage of the measurement mitigation from ignis
is the one thing blocking us from deprecating qiskit-ignis completely.
By embedding the code the quantum instance depends on inside
qiskit.utils users of QuantumInstance (and therefore qiskit.algorithms)
can construct and use measurement mitigation in it's current form. The
use of this migrated module is only supported for use with the
QuantumInstance class and is explicitly documented as internal/private
except for how it gets used by the QuantumInstance.

There is ongoing work to create a standardized mitigation API in Qiskit/qiskit#6485
and Qiskit/qiskit#6748, this does not preclude that work, but we should adapt this as
part of those efforts to use the standardized interface. Ideally this
would have been made a private interface and not exposed it as user facing
(in deference to the standardization effort), but unfortunately the
QuantumInstance expects classes of these classes as it's public interface
for selecting a mitigation technique which means users need to be able to
use the classes. However, as only the classes are public interfaces we
can adapt this as we come up with a standardized mitigation interface
and rewrite the internals of this and how the QuantumInstance leverages
mitigators to use the new interface.

A good follow-up here would be to adapt the mitigator selection kwarg to
deprecate the use of classes and then we can make things explicitly
private in the migrated code and wait for Qiskit/qiskit#6495 and Qiskit/qiskit#6748 to be ready
for our user facing API. I opted to not include that in this PR to
minimize changes to just what we migrated from ignis and update usage of
old ignis classes to rely on the migrated version.

* Update releasenotes/notes/ignis-mitigators-70492690cbcf99ca.yaml

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>

* Finish release notes

* Move API stability warning to the top

* Apply suggestions from code review

Co-authored-by: Jake Lishman <jake@binhbar.com>

* Remove unecessary test subclassing

* Fix skip logic in algorithm mitigation tests

* Fix exception handling on missing ignis in algorithms

* Assert deprecation message for ignis use mentions ignis

* Make filters module properly private

* Fix lint

Co-authored-by: Steve Wood <40241007+woodsp-ibm@users.noreply.github.com>
Co-authored-by: Jake Lishman <jake@binhbar.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: New Feature Include in the "Added" section of the changelog on hold Can not fix yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants