Re-organize files so QASM circuits live in a different directory than…

[jordandsullivan]

… the scripts that generate them

[jordandsullivan]

You can ignore the failed checks for now -- I'm doing some debugging on GitHub actions.

I went one direction with this @natestemen , but I'm open to other options, and Will may have a different intention in his original issue.

[natestemen]

@jordandsullivan there are a lot of seemingly unrelated changes here. I think commits from #124 got pulled into this PR. Can we separate those out before a review just to ensure everything is safe?

[jordandsullivan]

@natestemen Merged the delete old files branch. lmk if you have any more questions about changes here

[natestemen]

You'll need to merge main into this branch for that to be reflected here. Or use any of the other myriad of git techniques to apply changes here.

[jordandsullivan]

Adding explanatory comments for changes.

[jordandsullivan]

This is just converting the old notebook version to a script

[jordandsullivan]

Updating imports because of moved modules and new script (vs. notebook) for generating the qasm code

[jordandsullivan]

Converted this to a script (see below).

[jordandsullivan]

Updating import references for moved files. I could see an argument that it doesn't make sense to have the Qiskit and Cirq files inside the benchmarks/scripts module though

[natestemen]

I think my biggest question regarding this PR is should benchmarks live inside ucc? It seems natural for someone to run something like this.

import ucc

circuit = ucc.benchmarks.xyz_circuit(...)
ucc.compile(circuit)

I also think it would be great to have a script that is able to be run to generate the exact qasm files that live in qasm_circuits. Right now we have generate_qasm.py but it's not clear what exactly it outputs, and it does not write to the qasm_circuits directory.

[jordandsullivan]

I think my biggest question regarding this PR is should benchmarks live inside ucc? It seems natural for someone to run something like this.

import ucc

circuit = ucc.benchmarks.xyz_circuit(...)
ucc.compile(circuit)

I also think it would be great to have a script that is able to be run to generate the exact qasm files that live in qasm_circuits. Right now we have generate_qasm.py but it's not clear what exactly it outputs, and it does not write to the qasm_circuits directory.

I see, I think I can buy that. Having a way to import benchmarks in a runnable manner. Would you expect to need to specify which representation to load them in? I.e. cirq, Qiskit, etc?

[natestemen]

Specifying the frontend definitely makes sense. It's what we do in Mitiq. E.g.:

def generate_qpe_circuit(
    evalue_reg: int,
    input_gate: cirq.Gate = cirq.T,
    return_type: Optional[str] = None, # <- qiskit, cirq, pyquil, etc
) -> QPROGRAM: ...

[jordandsullivan]

Specifying the frontend definitely makes sense. It's what we do in Mitiq. E.g.:

def generate_qpe_circuit(
    evalue_reg: int,
    input_gate: cirq.Gate = cirq.T,
    return_type: Optional[str] = None, # <- qiskit, cirq, pyquil, etc
) -> QPROGRAM: ...

I'm fine with this, but I think it's beyond the scope of this issue. The ideas as I understand it is to separate the QASM files from the script that generates them. I honestly don't know what makes the most sense here.
If we want to move the scripts to ucc/ucc/scripts and keep the benchmarks as they are at ucc/benchmarks, I suppose that could work.

We could also move the entire benchmarks module into ucc/ucc/benchmarks to make them importable as you suggest above. It will be a bit of a pain to change all of the references to benchmarks in the benchmarks/scripts repo, and again I'm not sure it is in scope for this issue.

[natestemen]

New comments

Just to check my understanding here, the main change in this PR is moving code from benchmarks/generate_qasm.ipynb to benchmarks/scripts/generate_qasm.py.

Re-reading #45, my question is, does benchmarks/scripts/generate_qasm.py generate everything under benchmarks/circuits?

Unfortunately there isn't a lot of detail in the original issue to go on so it's hard to know exactly what the original problem is, but I'm pretty sure this change satisfies it. Maybe @willzeng should also take a look at this PR as issue author.

Regarding moving code into ucc/

I'm fine with [specifying the frontend], but I think it's beyond the scope of this issue.

Agreed.

move the entire benchmarks module into ucc/benchmarks

This still makes sense to me, since it is likely code we want to support and probably(?) add tests for.

[natestemen]

Any reason this file needs to be an executable?

[jordandsullivan]

Just to check my understanding here, the main change in this PR is moving code from benchmarks/generate_qasm.ipynb to benchmarks/scripts/generate_qasm.py.

Yes that is correct.

The main question is, was this issue intended to move the benchmarks into a module within ucc, i.e. accessible by calling ucc.benchmarks import qaoa etc.?

Or was the intention to just separate the QASM files from the scripts that generate them into a different directory, as I have done in this PR?