PRINT-INSTRUCTION-GENERIC fails to print gate applications with complex-valued parameters

[appleby]

Minimal reproduction case:

echo "RX(i) 0" | ./quilc
! ! ! Error: There is no applicable method for the generic function
               #<STANDARD-GENERIC-FUNCTION CL-QUIL::PRINT-INSTRUCTION-GENERIC (28)>
             when called with arguments
               (#C(0.0d0 1.0d0) #<SB-IMPL::STRING-OUTPUT-STREAM {100501CED3}>).
See also:
  The ANSI Standard, Section 7.6.6

The following files from the parser tests also tickle the bug:

tests/good-test-files/good-complex-complex-number.quil
tests/good-test-files/good-complex-params.quil
tests/good-test-files/good-simple-params.quil

For example:

$ ./quilc < tests/good-test-files/good-simple-params.quil
! ! ! Error: There is no applicable method for the generic function
               #<STANDARD-GENERIC-FUNCTION CL-QUIL::PRINT-INSTRUCTION-GENERIC (28)>
             when called with arguments
               (#C(-0.5d0 1.0d0) #<SB-IMPL::STRING-OUTPUT-STREAM {10054CC323}>).
See also:
  The ANSI Standard, Section 7.6.6

Note that these Quil programs can be parsed and printed just fine, as long as they don't pass through compiler-hook.

CL-USER> (quil::print-parsed-program (quil:parse-quil "RX(i) 0"))
RX(1.0i) 0
NIL
CL-USER> (quil::print-parsed-program
	  (quil:compiler-hook (quil:parse-quil "RX(i) 0")
			      (quil::build-nq-linear-chip 2)))
; Evaluation aborted on #<SB-PCL::NO-APPLICABLE-METHOD-ERROR {100212D953}>.

There is no applicable method for the generic function
  #<STANDARD-GENERIC-FUNCTION CL-QUIL::PRINT-INSTRUCTION-GENERIC (28)>
when called with arguments
  (#C(0.0d0 1.0d0)
   #<SB-IMPL::STRING-OUTPUT-STREAM {100212D163}>).
   [Condition of type SB-PCL::NO-APPLICABLE-METHOD-ERROR]
See also:
  Common Lisp Hyperspec, 7.6.6 [:section]

Restarts:
 0: [RETRY] Retry calling the generic function.
 1: [TRY-NEXT-COMPILER] Ignore this error and try the next compiler in the list.
 2: [RETRY] Retry SLIME REPL evaluation request.
 3: [*ABORT] Return to SLIME's top level.
 4: [ABORT] abort thread (#<THREAD "repl-thread" RUNNING {1001F31BC3}>)

Backtrace:
  0: ((:METHOD NO-APPLICABLE-METHOD (T)) #<STANDARD-GENERIC-FUNCTION CL-QUIL::PRINT-INSTRUCTION-GENERIC (28)> #C(0.0d0 1.0d0) #<SB-IMPL::STRING-OUTPUT-STREAM {100212D163}>) [fast-method]
  1: (SB-PCL::CALL-NO-APPLICABLE-METHOD #<STANDARD-GENERIC-FUNCTION CL-QUIL::PRINT-INSTRUCTION-GENERIC (28)> (#C(0.0d0 1.0d0) #<SB-IMPL::STRING-OUTPUT-STREAM {100212D163}>))
  2: ((:METHOD CL-QUIL::PRINT-INSTRUCTION-GENERIC (T NULL)) #C(0.0d0 1.0d0) #<unused argument>) [fast-method]
  3: ((:METHOD CL-QUIL::PRINT-INSTRUCTION-GENERIC (CL-QUIL:APPLICATION STREAM)) #<error printing GATE-APPLICATION {1002129CC3}> #<BROADCAST-STREAM {100002CD13}>) [fast-method]
  4: ((LABELS CL-QUIL::TRY-COMPILER :IN CL-QUIL::APPLY-TRANSLATION-COMPILERS) #<FUNCTION CL-QUIL::RX-TO-ZXZXZ>)
  5: (SB-KERNEL:%MAP-FOR-EFFECT-ARITY-1 #<CLOSURE (LABELS CL-QUIL::TRY-COMPILER :IN CL-QUIL::APPLY-TRANSLATION-COMPILERS) {100206A1FB}> #(#<FUNCTION CL-QUIL::PHASE-TO-RZ> #<FUNCTION CL-QUIL::RY-TO-XZX> #<F..
  6: (CL-QUIL::APPLY-TRANSLATION-COMPILERS #<RX(1.0i) 0> #<CL-QUIL::CHIP-SPECIFICATION of 2:1 objects> #<CL-QUIL::HARDWARE-OBJECT of order 0 {1001BF0E23}>)
  7: (CL-QUIL::FLUSH-1Q-INSTRUCTIONS-FORWARD #<CL-QUIL::CHIP-SCHEDULE {1002054863}> 0 (#<RX(1.0i) 0>) NIL NIL #<CL-QUIL:PARSED-PROGRAM {1001B4D323}>)
  8: ((LABELS CL-QUIL::FLUSH-1Q-INSTRUCTIONS-AFTER-WIRING :IN CL-QUIL::DO-GREEDY-TEMPORAL-ADDRESSING) 0)
  9: (CL-QUIL::DO-GREEDY-TEMPORAL-ADDRESSING (#<RX(1.0i) 0>) #<CL-QUIL::CHIP-SPECIFICATION of 2:1 objects> :ENVIRONS #<CL-QUIL:PARSED-PROGRAM {1001B4D323}> :INITIAL-REWIRING #S(CL-QUIL::REWIRING :L2P #(0 1..
 10: ((LABELS CL-QUIL::PROCESS-BLOCK :IN CL-QUIL:COMPILER-HOOK) #<CL-QUIL::BASIC-BLOCK ENTRY-BLK-658 len:1 in:0 out:term {1001CC9153}> NIL)
 11: (CL-QUIL:COMPILER-HOOK #<CL-QUIL:PARSED-PROGRAM {1001B4D323}> #<CL-QUIL::CHIP-SPECIFICATION of 2:1 objects> :PROTOQUIL NIL :REWIRING-TYPE NIL)
 12: (SB-INT:SIMPLE-EVAL-IN-LEXENV (CL-QUIL:COMPILER-HOOK (CL-QUIL:PARSE-QUIL "RX(i) 0") (CL-QUIL::BUILD-NQ-LINEAR-CHIP 2)) #<NULL-LEXENV>)
 13: (SB-INT:SIMPLE-EVAL-IN-LEXENV (CL-QUIL::PRINT-PARSED-PROGRAM (CL-QUIL:COMPILER-HOOK (CL-QUIL:PARSE-QUIL "RX(i) 0") (CL-QUIL::BUILD-NQ-LINEAR-CHIP 2))) #<NULL-LEXENV>)
 14: (EVAL (CL-QUIL::PRINT-PARSED-PROGRAM (CL-QUIL:COMPILER-HOOK (CL-QUIL:PARSE-QUIL "RX(i) 0") (CL-QUIL::BUILD-NQ-LINEAR-CHIP 2))))

[notmgsk]

Woof. Seems like a similar issue #257.

[appleby]

Likewise #272, which I am currently working on. All related to print-instruction bugs, at least.

I'm planning to tackle this one after #272.

[ecpeterson]

Quil tried to get a little more rigid about its typing last summer (cf. https://github.com/rigetti/quil/blob/master/spec/typed-memory.md ), and I think parameter values are forbidden from being complex numbers. However, the Quilt RFC ( https://github.com/rigetti/quil/tree/master/rfcs/analog ) wants complex parameter values in some of its new constructs.

This isn't to say you shouldn't work on this issue (especially since it might become more relevant when Quilt gets under way), but rather to explain why no user has reported the bug in the wild.

[appleby]

Good to know, thank you.

I will take a look in any case. If the fix to support complex params seems simple, maybe we go ahead and do it in anticipation of Quilt support (or maybe not yet).

If it looks like a bigger can of worms requiring medium-to-large changes, perhaps it makes sense to shelve it for now and maybe just update quilc to provide a better error message.

We'll see!

[appleby]

So it turns out that this instruction is actually failing to print in the guts of APPLY-TRANSLATION-COMPILERS when attempting to apply the RX-to-ZXZXZ translator. The translator fails and APPLY-TRANSLATION-COMPILERS attempts to print the instruction as part of the error message.

(restart-case
    (handler-case
        (let ((result (funcall compilation-method instruction)))
          (let ((*print-pretty* nil))
            (format *compiler-noise-stream* "APPLY-TRANSLATION-COMPILERS: Applying compiler ~a to ~a.~%"
                    compilation-method
                    (print-instruction instruction nil)))
          (dolist (instr result)
            (write-string "    " *compiler-noise-stream*)
            (print-instruction instr *compiler-noise-stream*)
            (terpri *compiler-noise-stream*))
          (return-from apply-translation-compilers result))
      (compiler-does-not-apply () nil))
  (try-next-compiler ()
    :report "Ignore this error and try the next compiler in the list."))

I was not paying close enough attention to the backtrace in my above example and incorrectly assumed that the program had made it through compiler-hook and was just falling over at the end in PRINT-PARSED-PROGRAM when emitting the final compiled program.

Given that this will require compiler changes (as opposed to a PRINT-INSTRUCTION bug fix), maybe the sensible thing to do is just to improve the error message for now, and hold off on adding complex-param support until it's actually needed as part of the Quilt RFC mentioned above? @ecpeterson @stylewarning @notmgsk What do folks think?

Note that this error is tangentially related to a code-review comment from earlier today, namely passing a complex param to build-gate winds up in the otherwise clause of capture-param, and so the bare complex value gets propagated.

(defun build-gate (operator params &rest qubits)
  "Shorthand function for constructing a GATE-APPLICATION object from QUIL-like input. OPERATOR must be a standard gate name."
  (check-type params list)
  (check-type qubits list)
  (assert (not (null qubits)))
  (flet ((capture-param (param)
           (typecase param
             (double-float
              (constant param))
             (memory-ref
              (make-delayed-expression nil nil param))
             (otherwise
              param)))
  ...

If I modify capture-param to box any number (as opposed to only double-float) in a constant, then compilation fails in a more obvious way in the compressor:

The value
  #C(-0.0d0 -0.5d0)
is not of type
  REAL
   [Condition of type TYPE-ERROR]
...
0: (CL-QUIL::COMPRESS-INSTRUCTIONS-IN-CONTEXT (#<RZ(pi/2) 0> #<RX(pi/2) 0> #<RZ(1.0i) 0> #<RX(-pi/2) 0> #<RZ(-pi/2) 0>) #<CL-QUIL::CHIP-SPECIFICATION of 1:0 objects> #S(CL-QUIL::COMPRESSOR-CONTEXT :AQVM ..
  1: (CL-QUIL::COMPRESS-INSTRUCTIONS-WITH-POSSIBLY-UNKNOWN-PARAMS (#<RZ(pi/2) 0> #<RX(pi/2) 0> #<RZ(1.0i) 0> #<RX(-pi/2) 0> #<RZ(-pi/2) 0>) #<CL-QUIL::CHIP-SPECIFICATION of 1:0 objects> #S(CL-QUIL::COMPRES..
  2: ((LABELS CL-QUIL::TRANSITION-GOVERNOR-STATE :IN CL-QUIL::COMPRESS-INSTRUCTIONS) 0 0 :FLUSHING NIL)
  3: (CL-QUIL::COMPRESS-INSTRUCTIONS (#<RZ(pi/2) 0> #<RX(pi/2) 0> #<RZ(1.0i) 0> #<RX(-pi/2) 0> #<RZ(-pi/2) 0>) #<CL-QUIL::CHIP-SPECIFICATION of 1:0 objects> :PROTOQUIL T)
  4: ((LABELS CL-QUIL::PROCESS-BLOCK :IN CL-QUIL:COMPILER-HOOK) #<CL-QUIL::BASIC-BLOCK ENTRY-BLK-727 len:1 in:0 out:term {100DD7EDD3}> NIL)
  5: (CL-QUIL:COMPILER-HOOK #<CL-QUIL:PARSED-PROGRAM {100DD7C143}> #<CL-QUIL::CHIP-SPECIFICATION of 1:0 objects> :PROTOQUIL NIL :REWIRING-TYPE NIL)

[ecpeterson]

:) This is a cascade of sloppy typechecking. The parser didn't stop you from passing i as an argument to RX (even though the resulting matrix isn't even unitary), the compiler unboxed it and fed it to build-gate as a parameter, build-gate saw that it wasn't real and put it back into a new instruction as an unboxed number, the native instruction predicate saw that everything of the form "RZ (_) 0" is admissible and so didn't bother unpacking the parameters, the resulting instruction sequence got fed to the compressor, the compressor did a bunch of sorting based only on the arguments, the compressor tried to multiply the matrix representations of these things together, and (finally we hit a piece of code that Robert wrote) the code responsible for building the matrix representation of RZ(i) 0 ultimately threw a fit.

[ecpeterson]

(j/k Robert also wrote the parser)

[appleby]

This is why I like excessively checking types everywhere like some kind of paranoiac :-)

Were complex params previously supported? What has me confused is that these are explicitly tested and permitted in the parser tests (presumably with complex params that at least result in unitary matrices, unlike my silly RX(i) 0 example).

Perhaps the way to go would be to update the parser to complain early, then move these tests cases from good-test-files ---> bad-test-files?

[ecpeterson]

They were previously supported, but it was never a good idea: it's definitely possible to pick parameters in

DEFGATE U(%a, %b, %c, %d):
    %a, %b
    %c, %d

so that the the resulting gate is unitary, but (1) the pattern of legal values is quite nonobvious and (2) an illegal choice of parameters was blamed on the user and resulted in undefined behavior. What happens in practice is more like

DEFGATE RX(%theta):
    cos(%theta), -i*sin(%theta)
    -i*sin(%theta), cos(%theta)

where the pattern is obvious—%theta is required to be real, or else it's hopeless—and, after enforcing that constraint, there's no chance for bad user input. And so we made the decision that classical memory should only support REAL value types, and (I thought) that parameters were required to be real too. If you truly needed an escape hatch, you could still write

DEFGATE U(%ra, %ia, %rb, %ib, %rc, %ic, %rd, %id):
    %ra+i*%ia, %rb+i*%ib
    %rc+i*%ic, %rd+i*%id

but you really have to go out of your way to do such a silly thing.

I'd strongly prefer that we stick with this ^ and remove support for complex parameters in the parser (and the associated tests), but it ought to be (re-)sanctioned by @stylewarning.

[stylewarning]

I would be happy if we constrained parameters to just be real as a matter of practicality.

[appleby]

Closing this as resolved by #608.

Attempting to pass complex-valued gate parameters now raises a more readable error:

echo "RX(i) 0" | ./quilc --check-sdk-version=false
! ! ! Error: Complex-valued gate parameter #C(0.0d0 1.0d0) not permitted