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.

Sign up for GitHub

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

Jump to bottom

Parametric compilation broken #667

Closed
notmgsk opened this issue Aug 4, 2020 · 5 comments
Closed

Parametric compilation broken #667

notmgsk opened this issue Aug 4, 2020 · 5 comments
Assignees
@colescott @notmgsk
Labels
bug Something isn't working

Comments

@notmgsk
Copy link
Member

notmgsk commented Aug 4, 2020

For some qubits on Aspen-8 the following program raises a compiler-does-not-apply error.

import pyquil
qc = pyquil.get_qc('Aspen-8', as_qvm=True)
qc.compile(pyquil.Program("""DECLARE t REAL
RX(((1.0)*t[0])) 3"""))

RPCError: Unhandled error in host program:
Condition CL-QUIL::COMPILER-DOES-NOT-APPLY was signalled.

@colescott
@notmgsk notmgsk added the bug Something isn't working label Aug 4, 2020
@stylewarning
Copy link
Member

Was the "broken after" determined by bisection?

@notmgsk
Copy link
Member Author

notmgsk commented Aug 4, 2020

Yes

@notmgsk
Copy link
Member Author

notmgsk commented Aug 4, 2020 •
edited
Loading

Possibly a red herring, however. I have a different program that causes the issue before #655

H 1
H 2
H 3
H 4
H 5
DECLARE ro BIT[5]
DECLARE theta REAL[2]
CPHASE(theta[0]) 1 2
CPHASE(theta[0]) 2 3
CPHASE(theta[0]) 3 4
CPHASE(theta[0]) 4 5
RX(theta[1]) 1
RX(theta[1]) 2
RX(theta[1]) 3
RX(theta[1]) 4
RX(theta[1]) 5
MEASURE 1 ro[0]
MEASURE 2 ro[1]
MEASURE 3 ro[2]
MEASURE 4 ro[3]
MEASURE 5 ro[4]
PRAGMA INITIAL_REWIRING "NAIVE"

@notmgsk notmgsk changed the title Parametric compilation broken after #655 Parametric compilation broken Aug 4, 2020
@notmgsk notmgsk self-assigned this Aug 4, 2020
stylewarning added a commit to stylewarning/quilc that referenced this issue Aug 5, 2020
@stylewarning
ensure applicable symbolic-accepting compilers are included
87b2099 
When we changed the way occ-tbl metrics were calculated, it caused
quilc to find particularly good collections of compilers ("compiler
paths") for certain gate sets. These particularly good compiler paths
were better than before! But there was a hitch... the better compilers
were also less flexible in what they matched against, though that
wasn't apparent from the binding patterns themselves. In particular,
in some cases, the better compilers were ones that also did _not_
accept symbolic parameters (e.g., a memory reference). This means
these better compilers failed to take into account inputs like
`RX(theta) 0` even though the pattern `RX(_) _` was compilable.

The fix in this commit has two facets:

    - First, we use heuristics to determine if a compiler can accept
      symbolic parameters. This heuristic can be improved, but works
      fine for one-parameter compilers, which are the real-world cases
      that failed.

    - Second, even if we find a better compiler path when warming the
      chip spec, as long as a symbolic-accepting path was found, it's
      included in the list of compilers. This may still fail, if the
      entire path doesn't allow symbolic compilers. This was too
      complicated to check for this commit with too little gain.

This fixes both issues reported in issue quil-lang#667.
@stylewarning stylewarning mentioned this issue Aug 5, 2020
Merged
stylewarning added a commit to stylewarning/quilc that referenced this issue Aug 5, 2020
@stylewarning
ensure applicable symbolic-accepting compilers are included
6dd33df 
When we changed the way occ-tbl metrics were calculated, it caused
quilc to find particularly good collections of compilers ("compiler
paths") for certain gate sets. These particularly good compiler paths
were better than before! But there was a hitch... the better compilers
were also less flexible in what they matched against, though that
wasn't apparent from the binding patterns themselves. In particular,
in some cases, the better compilers were ones that also did _not_
accept symbolic parameters (e.g., a memory reference). This means
these better compilers failed to take into account inputs like
`RX(theta) 0` even though the pattern `RX(_) _` was compilable.

The fix in this commit has two facets:

    - First, we use heuristics to determine if a compiler can accept
      symbolic parameters. This heuristic can be improved, but works
      fine for one-parameter compilers, which are the real-world cases
      that failed.

    - Second, even if we find a better compiler path when warming the
      chip spec, as long as a symbolic-accepting path was found, it's
      included in the list of compilers. This may still fail, if the
      entire path doesn't allow symbolic compilers. This was too
      complicated to check for this commit with too little gain.

This fixes both issues reported in issue quil-lang#667.
notmgsk pushed a commit that referenced this issue Aug 16, 2020
@stylewarning
ensure applicable symbolic-accepting compilers are included (#669)
b6ceece 
When we changed the way occ-tbl metrics were calculated, it caused
quilc to find particularly good collections of compilers ("compiler
paths") for certain gate sets. These particularly good compiler paths
were better than before! But there was a hitch... the better compilers
were also less flexible in what they matched against, though that
wasn't apparent from the binding patterns themselves. In particular,
in some cases, the better compilers were ones that also did _not_
accept symbolic parameters (e.g., a memory reference). This means
these better compilers failed to take into account inputs like
`RX(theta) 0` even though the pattern `RX(_) _` was compilable.

The fix in this commit has two facets:

    - First, we use heuristics to determine if a compiler can accept
      symbolic parameters. This heuristic can be improved, but works
      fine for one-parameter compilers, which are the real-world cases
      that failed.

    - Second, even if we find a better compiler path when warming the
      chip spec, as long as a symbolic-accepting path was found, it's
      included in the list of compilers. This may still fail, if the
      entire path doesn't allow symbolic compilers. This was too
      complicated to check for this commit with too little gain.

This fixes both issues reported in issue #667.
@stylewarning
Copy link
Member

@notmgsk can you confirm your test case is fixed?

@notmgsk
Copy link
Member Author

notmgsk commented Aug 19, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@colescott colescott

@notmgsk notmgsk

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@stylewarning @colescott @notmgsk