-
Notifications
You must be signed in to change notification settings - Fork 57
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
Accelerate APPLY-QUBIT-PERMUTATION. #179
base: master
Are you sure you want to change the base?
Conversation
0197001
to
6b4158e
Compare
6b4158e
to
6afe4c8
Compare
0842977
to
00c4b62
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small stuff
57eba3b
to
f2619b8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
more
(declare (type qubit-index tau)) | ||
|
||
;; Swap bits 0 and TAU in ADDRESS. | ||
(let ((x (logxor (logand address 1) (logand (ash address (- tau)) 1)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about (rotatef (ldb (byte 1 ...) address) (ldb (byte 1 ...) address))
?
(setf ldb)
is a valid function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's exactly how it was before but the above produces tighter assembly code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, can you make a comment with #+#:slightly-slower (rotatef ...)
or so just to demo something equivalent and a lot more readable?
(values qvm:amplitude-address)) | ||
|
||
;; Swap bits 0 and TAU in ADDRESS. | ||
(let ((x (logxor (logand ,address 1) (logand (ash ,address ,minus-tau) 1)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see rotatef comment above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here. Perhaps factor it out into an inlined function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the lookup table is a neat hack
;;; more than three times faster than the equivalent application of a | ||
;;; PERMUTATION-GENERAL object). | ||
|
||
(defconstant +qubit-index-length+ (ceiling (log qvm::+max-nat-tuple-cardinality+ 2)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use integer-length
appropriately
(defconstant +qubit-index-length+ (ceiling (log qvm::+max-nat-tuple-cardinality+ 2)) | ||
"Number of bits required to represent a qubit index.") | ||
|
||
(defconstant +max-number-of-transpositions+ (* 2 #.qvm::+max-nat-tuple-cardinality+) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need for #.
"Upper bound on the number of transpositions defining an arbitrary permutation.") | ||
|
||
(deftype qubit-index () | ||
'(unsigned-byte #.+qubit-index-length+)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
quasiquoting looks a little cleaner
|
||
(deftype transposition () | ||
'(or null (cons alexandria:non-negative-fixnum | ||
alexandria:non-negative-fixnum))) | ||
'(or null (cons qubit-index qubit-index))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how can a null transposition show up?
(transpositions (permutation-transpositions permutation))) | ||
(cond | ||
((and (null domain) (null codomain)) nil) | ||
((and (= 1 (length domain)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(null (cdr domain))
(the qvm:amplitude-address (first codomain))))) | ||
(make-instance 'permutation-transposition :tau max-index :size (1+ max-index))) | ||
((and (= 2 (length domain)) | ||
(null (set-difference domain codomain)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you want difference or intersection?
(loop :for a :of-type qubit-index :in domain | ||
:for b :of-type qubit-index :across codomain | ||
:unless (= a b) :do | ||
(pushnew (cons a b) transpositions :test #'equal))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would write a transposition=
function and use it here, even if it's as simple as
(defun transposition= (a b)
(and (= (car a) (car b)) (= (cdr a) (cdr b))))
(declare (type qubit-index tau)) | ||
|
||
;; Swap bits 0 and TAU in ADDRESS. | ||
(let ((x (logxor (logand address 1) (logand (ash address (- tau)) 1)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, can you make a comment with #+#:slightly-slower (rotatef ...)
or so just to demo something equivalent and a lot more readable?
(values qvm:amplitude-address)) | ||
|
||
;; Swap bits 0 and TAU in ADDRESS. | ||
(let ((x (logxor (logand ,address 1) (logand (ash ,address ,minus-tau) 1)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here. Perhaps factor it out into an inlined function
(byte ,num-bits 0) | ||
,address))))) | ||
|
||
(defun compile-qubit-permutation (permutation) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:')
Part of #177.