rigetti / pyquil

A Python library for quantum programming using Quil.
http://docs.rigetti.com
Apache License 2.0
1.4k stars 342 forks source link

feat: Add utility for filtering Programs and a method for removing Quil-T instructions #1718

Closed MarquessV closed 8 months ago

MarquessV commented 9 months ago

Description

Closes #1684

Checklist

rigetti-githubbot commented 9 months ago

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
7099 6225 88% 87% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
pyquil/quil.py 84% 🟢
TOTAL 84% 🟢

updated for commit: fffbf85 by action🐍

MarquessV commented 9 months ago

Hey @bramathon and @mhodson-rigetti, wanted to get y'alls input before moving forward with this.

This PR would give users a utility to easily remove Quil-T instructions from a program as well as a more general method for filtering any kind of instruction from a program.

However, this PR does not automatically remove Quil-T instructions before QVM execution. Instead, we document a pattern users can use to remove them themselves based on their execution target. We want to avoid potentially surprising behavior and to start establishing patterns around how to manage the differences between QVM and QPU execution. pyQuil has always tried to sweep these difficulties under the rug, but it's caused us trouble here, and since I'd guess that QVM and QPU behavior are going to continue diverging, it feels like a risk to continue trying to hide those differences.

All of that said, I do see the convenience in removing the Quil-T instructions automatically and maybe this isn't the right feature or time to start emphasizing these differences, so still happy to do that if this feels like the wrong approach to you all. Just want to make sure we're aligned on this issue so we can move forward.

MarquessV commented 8 months ago

I'm going to open this up for general review now. CC: @bramathon and @mhodson-rigetti, no worries if this slipped by the radar, just want to make sure these changes don't get stale.

mhodson-rigetti commented 8 months ago

Change set looks good to me, besides the comment on example usage.

bramathon commented 8 months ago

Hey @bramathon and @mhodson-rigetti, wanted to get y'alls input before moving forward with this.

This PR would give users a utility to easily remove Quil-T instructions from a program as well as a more general method for filtering any kind of instruction from a program.

However, this PR does not automatically remove Quil-T instructions before QVM execution. Instead, we document a pattern users can use to remove them themselves based on their execution target. We want to avoid potentially surprising behavior and to start establishing patterns around how to manage the differences between QVM and QPU execution. pyQuil has always tried to sweep these difficulties under the rug, but it's caused us trouble here, and since I'd guess that QVM and QPU behavior are going to continue diverging, it feels like a risk to continue trying to hide those differences.

All of that said, I do see the convenience in removing the Quil-T instructions automatically and maybe this isn't the right feature or time to start emphasizing these differences, so still happy to do that if this feels like the wrong approach to you all. Just want to make sure we're aligned on this issue so we can move forward.

I believe the source of the problem is that the QuantumComputer object is designed to act as both a QPU and a QVM, with only a flag indicating which backend is being used. The implicit promise to the user is that this object can run quil programs. But in reality, the QVM supports quil, plus a special set of pragmas, while the qpu supports quil and quil-T. If it is the case that the QPU-supported specification will further diverge from the QVM-supported specification, we are setting users up for problems by treating them as identical backends. Perhaps the QPU and QVM should be different classes in this case.

However, in my experience a drop-in Quil simulator is precisely what users want and I don't see a problem with simply ignoring the unsupported Quil-T timing instructions by default. Perhaps the issue is that this should not happen in the client, but rather in the QVM.