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

Cannot use resets in a circuit #5736

Closed
nonhermitian opened this issue Jan 28, 2021 · 7 comments
Closed

Cannot use resets in a circuit #5736

nonhermitian opened this issue Jan 28, 2021 · 7 comments
Assignees
Labels
bug Something isn't working priority: high stable backport potential The bug might be minimal and/or import enough to be port to stable

Comments

@nonhermitian
Copy link
Contributor

Information

  • Qiskit Terra version: latest
  • Python version:
  • Operating system:

What is the current behavior?

     ┌─────────┐     ┌─┐
q_0: ┤ unitary ├─|0>─┤M├
     └─────────┘     └╥┘
c: 1/═════════════════╩═
                      0 

gives:

~/miniconda3/envs/qiskit/lib/python3.7/site-packages/qiskit/quantum_info/operators/operator.py in _append_instruction(self, obj, qargs)
    526             # cannot compose this gate and raise an error.
    527             if obj.definition is None:
--> 528                 raise QiskitError('Cannot apply Instruction: {}'.format(obj.name))
    529             if not isinstance(obj.definition, QuantumCircuit):
    530                 raise QiskitError('Instruction "{}" '

QiskitError: 'Cannot apply Instruction: reset'

even if the backend supports it. The code I am using used to work in previous versions.

Steps to reproduce the problem

What is the expected behavior?

Suggested solutions

@nonhermitian nonhermitian added the bug Something isn't working label Jan 28, 2021
@mtreinish
Copy link
Member

What operations are you running on that circuit to raise the error? That exception is being raised from trying to create an operator from something (I assume the circuit given it's erroring on reset). We probably just need to add special handling to the operator constructor to deal with reset instructions.

As far as I know everything in the circuit -> transpile -> assemble -> execute path works for reset (although there are some complaints about resets being removed during transpilation #5127, although that case isn't a hard failure it just optimizes away resets during a transpile)

@nonhermitian
Copy link
Contributor Author

It is a random 1Q unitary followed by a reset and meas (as the circuit shows), then passed to device using execute. I can confirm the exact same code works on 0.23.2.

@mtreinish
Copy link
Member

mtreinish commented Jan 28, 2021

Ok, that's what I needed to know, it's being raised by a transpile (that wasn't in the orriginal issue). This is actually a duplicate of #5543 (although it should be fixed by #5570) and we unfortunately backported a regression in 0.16.2 with #5655 (which I thought should have included the fix). Which means we'll need to fix it for real on the stable branch and push out a terra 0.16.3.

@mtreinish
Copy link
Member

FWIW, this should work fine on master, it appears to just be a bug in that backport :(

@mtreinish mtreinish added priority: high stable backport potential The bug might be minimal and/or import enough to be port to stable labels Jan 28, 2021
@nonhermitian
Copy link
Contributor Author

Ok cool. Getting a fix out would be nice since reset is a new(ish) feature we would like to highlight.

@mtreinish
Copy link
Member

Yeah, I've got the patch ready, just adding a test now. As soon as that merges to stable/0.16 we'll prepare and push out a 0.16.3 release

mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Jan 28, 2021
This commit fixes a bug introduced in the backport PR Qiskit#5655. On the
current master branch the bugfix that was being backported relies on a
feature in newer versions of retworkx. Since we can't bump the minimum
retworkx version required on a stable branch that functionality was
rewritten in the dagcircuit class directly. However, that implementation
included a typo which is causing non-gates and atomic instructions to be
included in collected runs which is not correct behavior and causes
errors to be raised. This commit corrects the bug in the backport and
adds a test to ensure the regression is not present.

This is a direct commit to the stable branch and not a backport since
this bug is not present on master and was only introduced through a bad
backport.

Fixes Qiskit#5736
@mtreinish mtreinish self-assigned this Jan 28, 2021
mergify bot pushed a commit that referenced this issue Jan 28, 2021
This commit fixes a bug introduced in the backport PR #5655. On the
current master branch the bugfix that was being backported relies on a
feature in newer versions of retworkx. Since we can't bump the minimum
retworkx version required on a stable branch that functionality was
rewritten in the dagcircuit class directly. However, that implementation
included a typo which is causing non-gates and atomic instructions to be
included in collected runs which is not correct behavior and causes
errors to be raised. This commit corrects the bug in the backport and
adds a test to ensure the regression is not present.

This is a direct commit to the stable branch and not a backport since
this bug is not present on master and was only introduced through a bad
backport.

Fixes #5736
@mtreinish
Copy link
Member

This was fixed in #5740 which has merged and been released as 0.16.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority: high stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
None yet
Development

No branches or pull requests

2 participants