Raise TypeError when .run() is called with non-compiled Program #799

notmgsk · 2019-02-06T21:51:31Z

Couple things.

The test is currently performed in QuantumComputer.run(). Should
the test be performed in QPU.run()? I chose QC.run() because that is
where the executable is provided and will show up in the
backtrace.
That test might not be the best it could be. Since we're not
running on a QPU, we can't instantiate a true QPU object -- is the
way I've mocked it appropriate and safe?

Closes #740.

Have a good day :}

Couple things. 1. The test is currently performed in QuantumComputer.run(). Should the test be performed in QPU.run()? I chose QC.run() because that is where the `executable` is provided and will show up in the backtrace. 2. That test might not be the best it could be. Since we're not running on a QPU, we can't instantiate a true `QPU` object -- is the way I've mocked it appropriate and safe? Have a good day :}

mpharrigan · 2019-02-07T01:13:52Z

pyquil/api/_quantum_computer.py

@@ -144,6 +144,13 @@ def run(self, executable: Executable,
            are a list of floats or integers.
        :return: A numpy array of shape (trials, len(ro-register)) that contains 0s and 1s.
        """
+        # This prevents a common error where users expect QVM.run()
+        # and QPU.run() to be interchangeable. QPU.run() needs the
+        # supplied executable to have been compiled, QVM.run() does not.


some more color here: if you construct a qvm with a topology, the code will assume you care about realism and will error if you pass a program instead of an executable. However, we thought that would be annoying if you didn't really care about compiling/realism so if you construct a {n}q-qvm without a topology it will accept Programs

>> get_qc('2q-qvm').run(program) # fine >> get_qc('Aspen-1-2Q-A-qvm').run(program) # error! you need to compile

this might be more confusing than it's worth

mpharrigan · 2019-02-07T01:14:29Z

pyquil/api/_quantum_computer.py

+        # This prevents a common error where users expect QVM.run()
+        # and QPU.run() to be interchangeable. QPU.run() needs the
+        # supplied executable to have been compiled, QVM.run() does not.
+        if isinstance(self.qam, QPU) and isinstance(executable, Program):


why not put this check in the QPU class?

From above:

The test is currently performed in QuantumComputer.run(). Should the test be performed in QPU.run()? I chose QC.run() because that is where the executable is provided and will show up in the backtrace.

I think if you put it in QPU it will still show up in the backtrace, just one level up

mpharrigan · 2019-02-07T01:17:12Z

pyquil/tests/test_quantum_computer.py

+    # This test might need some more knowledgeable eyes. Not sure how
+    # to best mock a qpu.
+    qc = get_qc('1q-qvm')
+    qc.qam = QPU(endpoint='tcp://not-needed:00000',


I think the legit way to mock this would be to mock out QPU.client.call which is the rpcq thing that actually communicates with the quantum hardware. Maybe the rpcq repo has some useful test fixtures we could repurpose here?

lcapelluto

A couple minor comments you might want to address. Nice change :)

lcapelluto · 2019-02-07T18:53:49Z

pyquil/tests/test_quantum_computer.py

@@ -5,7 +5,7 @@
 import pytest

 from pyquil import Program, get_qc, list_quantum_computers
-from pyquil.api import QVM, QuantumComputer, local_qvm
+from pyquil.api import QVM, QPU, QuantumComputer, local_qvm


it doesn't look like this is used anywhere

lcapelluto · 2019-02-07T18:58:18Z

pyquil/api/_qpu.py

@@ -120,6 +121,13 @@ def run(self):

        :return: The QPU object itself.
        """
+        # This prevents a common error where users expect QVM.run()
+        # and QPU.run() to be interchangeable. QPU.run() needs the
+        # supplied executable to have been compiled, QVM.run() does not.


This is good, I don't remember what the error would be otherwise, but this error message is certainly better.

Ideally, QPU and QVM should be interchangeable.
I think a warning is raised when running on QVM without calling .compile, or maybe only in some cases. Maybe we can consider rewording this comment to encourage users to call .compile even when targeting the QVM, to prevent people from running into this issue in the first place.

the full story here: #799 (comment)

mpharrigan · 2019-02-07T21:02:19Z

pyquil/tests/test_quantum_computer.py

@@ -5,7 +5,7 @@
 import pytest

 from pyquil import Program, get_qc, list_quantum_computers
-from pyquil.api import QVM, QuantumComputer, local_qvm
+from pyquil.api import QVM, QPU, QuantumComputer, local_qvm


Suggested change

from pyquil.api import QVM, QPU, QuantumComputer, local_qvm

from pyquil.api import QVM, QuantumComputer, local_qvm

notmgsk requested review from mpharrigan, lcapelluto and ecpeterson and removed request for mpharrigan and lcapelluto February 6, 2019 21:51

notmgsk force-pushed the fix/better-error-for-unexpected-program branch from 89832d3 to 581a41c Compare February 6, 2019 22:16

mpharrigan reviewed Feb 7, 2019

View reviewed changes

Move isinstance to QPU.run (and tests)

0df72ce

lcapelluto approved these changes Feb 7, 2019

View reviewed changes

mpharrigan reviewed Feb 7, 2019

View reviewed changes

Remove useless QPU import

b1420db

mpharrigan merged commit 61d4d6e into master Feb 11, 2019

mpharrigan deleted the fix/better-error-for-unexpected-program branch February 11, 2019 18:54

karalekas added this to the v2.4.0 milestone Feb 14, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Raise TypeError when .run() is called with non-compiled Program #799

Raise TypeError when .run() is called with non-compiled Program #799

notmgsk commented Feb 6, 2019

mpharrigan Feb 7, 2019

mpharrigan Feb 7, 2019

notmgsk Feb 7, 2019 •

edited

Loading

mpharrigan Feb 7, 2019

mpharrigan Feb 7, 2019

lcapelluto left a comment

lcapelluto Feb 7, 2019

lcapelluto Feb 7, 2019

mpharrigan Feb 7, 2019

mpharrigan Feb 7, 2019

	from pyquil.api import QVM, QPU, QuantumComputer, local_qvm
	from pyquil.api import QVM, QuantumComputer, local_qvm

Raise TypeError when .run() is called with non-compiled Program #799

Raise TypeError when .run() is called with non-compiled Program #799

Conversation

notmgsk commented Feb 6, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

notmgsk Feb 7, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lcapelluto left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

notmgsk Feb 7, 2019 •

edited

Loading