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

marginal_counts is broken for pulse results #6430

Closed
taalexander opened this issue May 17, 2021 · 8 comments · Fixed by #8099
Closed

marginal_counts is broken for pulse results #6430

taalexander opened this issue May 17, 2021 · 8 comments · Fixed by #8099
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@taalexander
Copy link
Contributor

Information

  • Qiskit Terra version: 0.18.0
  • Python version: 3.9
  • Operating system: OSx

What is the current behavior?

marginal_counts is broken for pulse jobs. I

Steps to reproduce the problem

from qiskit import pulse, QuantumCircuit, schedule, transpile, execute, IBMQ
from qiskit.result import marginal_counts


IBMQ.load_account()
provider = IBMQ.get_provider(hub='ibm-q', group='open', project='main')
backend = provider.get_backend('ibmq_armonk')

qc = QuantumCircuit(1,1)
qc.measure(0, 0)

job = backend.run(schedule(transpile(qc, backend), backend))
marginal_counts(job.result(), indices=[0])

Gives output

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-6-8e897d2c2a54> in <module>
     11 
     12 job = backend.run(schedule(transpile(qc, backend), backend))
---> 13 marginal_counts(job.result(), indices=[0])

~/opt/anaconda3/envs/python39/lib/python3.9/site-packages/qiskit/result/utils.py in marginal_counts(result, indices, inplace, format_marginal)
     55             experiment_result.data.counts = new_counts_hex
     56             experiment_result.header.memory_slots = len(indices)
---> 57             csize = experiment_result.header.creg_sizes
     58             experiment_result.header.creg_sizes = _adjust_creg_sizes(csize, indices)
     59         return result

AttributeError: 'QobjExperimentHeader' object has no attribute 'creg_sizes'

What is the expected behavior?

Suggested solutions

It seems like it is assuming creg will be present in the header. This is not true for pulse jobs which do not have classical registers and therefore this field won't be present. The fix should make marginal_counts work when the header is empty.

@taalexander taalexander added bug Something isn't working good first issue Good for newcomers labels May 17, 2021
@mtreinish
Copy link
Member

What happens if you do marginal_counts(job.result.get_counts()), indices=[0])? That's my normal workflow for using marginal_counts() because using it with results ends up deepcopying and making it too slow.

Of course, that's independent of this bug though we should fix the function for pulse results too.

@taalexander
Copy link
Contributor Author

taalexander commented May 17, 2021 via email

rithikaadiga added a commit to rithikaadiga/qiskit-terra that referenced this issue May 30, 2021
…s if creg_sizes is an attribute in the object and executes related code, only if it is.
@timsinashok
Copy link

I am new to quantum computing and would love to work on this issue.

@javabster
Copy link
Contributor

Thanks @a-freakish! Assigning to you 😄

@mtreinish
Copy link
Member

This issue already has an open pr in progress: #6486

@chetmurthy
Copy link
Contributor

The referenced pr #6486 appears to be closed (b/c no further work done) ? Should someone pick up this PR and finish it?

@1ucian0
Copy link
Member

1ucian0 commented May 21, 2022

yes please @chetmurthy ! Remember to respect the @rithikaadiga's authorship in branch off their code. Shall I assign you?

@chetmurthy
Copy link
Contributor

chetmurthy commented May 21, 2022

Luciano, I'm new to the org, and to QC. But this seems sufficiently "just a software problem" that, yes, I think I should have a go at it. You can mark me down for working on it.

@mergify mergify bot closed this as completed in #8099 Jun 7, 2022
mergify bot added a commit that referenced this issue Jun 7, 2022
)

* fix marginal_counts (which failed on pulse backend)

this is code copied from @rithikaadiga 's branch, but it's so small I
just copied it, b/c didn't want to deal with applying all the changes
since in the main trunk.

* add release-note

* Update qiskit/result/utils.py

Co-authored-by: Luciano Bello <[email protected]>

* Update releasenotes/fix-marginal_counts-on-pulse-backend.yaml

Co-authored-by: Luciano Bello <[email protected]>

* Update releasenotes/fix-marginal_counts-on-pulse-backend.yaml

Co-authored-by: Luciano Bello <[email protected]>

* fix formatting (black)

Co-authored-by: rithikaadiga <[email protected]>

* minor cleanup to suggested fix.

# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
#
# On branch fix-marginal_counts-on-pulse-backend
# Your branch is up to date with 'my-fork/fix-marginal_counts-on-pulse-backend'.
#
# Changes to be committed:
#	modified:   qiskit/result/utils.py
#

* Update releasenotes/fix-marginal_counts-on-pulse-backend.yaml

Co-authored-by: Luciano Bello <[email protected]>

* Update releasenotes/fix-marginal_counts-on-pulse-backend.yaml

Co-authored-by: Luciano Bello <[email protected]>

* Update test/python/result/test_result.py

Co-authored-by: Luciano Bello <[email protected]>

* redo test per suggestion from @1ucian0

# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
#
# On branch fix-marginal_counts-on-pulse-backend
# Your branch is up to date with 'my-fork/fix-marginal_counts-on-pulse-backend'.
#
# Changes to be committed:
#	modified:   test/python/result/test_result.py
#

* Update test/python/result/test_result.py

Co-authored-by: Luciano Bello <[email protected]>

Co-authored-by: Luciano Bello <[email protected]>
Co-authored-by: rithikaadiga <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
6 participants