Directory structure for benchmark code

[bachase]

This is a brief proposal for changing the directory structure of the repository to prevent undesired cross-module dependencies but still enable fast development and iteration.

Current State

The current directory structure of the ucc repository is

ucc
├── benchmarks
│   ├── qasm_circuits
│   ├── results
│   └── scripts
├── docs
│   └── source
└── ucc
    ├── tests
    ├── transpiler_passes
    └── transpilers

Here both ucc and benchmarks are python packages. However in switching to poetry from setuptools, we no longer distribute the benchmarks module in the python wheel. This is preferred, as users might be confused to have benchmarks as an available package even though they only installed ucc.

But this causes two issues:

1. As identified in Doc example using missing function count_multi_qubit_gates_qiskit #204 , an example in the docs imports helper functions from the benchmarks module, but these won't be available
2. Some ucc unit tests also import from the benchmarks module. These would fail for users that want to run tests directly on the python packages. (Tangentially related, I think the recommend pattern, as shown in the poetry docs is to hoist the unit tests up a level and not distribute them as a submodule of ucc.)

Proposal

Long term, I think @nathanshammah has the right idea of moving benchmarking to a separate repository (perhaps even metriq) as users of ucc won't need them. But in the alpha phase of the project, I believe there is a benefit of having this monorepo-like structure, where it will be easier to iterate on benchmark and core ucc code together.

1. Keep benchmarks and ucc folders as siblings, but hoist the ucc/tests module up a level.
2. Move the function referenced in the docs to a new ucc.helper or ucc.common submodule (not sure on name).

   ucc/docs/source/user_guide.rst

   Line 33 in 9f5e96c

   from benchmarks.scripts.common import count_multi_qubit_gates_qiskit

3. Add a pre-commit hook/lint check (as part of adding those generally) which prevents any code in ucc module importing from the benchmarks file.

Open Questions

1. The unit test below imports two helpers from the benchmark module. Should those helpers be moved as well? We can leave as is if we no longer "ship" the unit test code, but it feels off to have this cross dependency

   ucc/ucc/tests/test_compile.py

   Line 11 in 9f5e96c

   from benchmarks.scripts import qcnn_circuit, random_clifford_circuit

[nathanshammah]

Many thanks @bachase for the nice summary. Actually your clear explanation makes me lean toward having benchmarks under ucc/ucc in this alpha stage since as a user I'd like to test them and have them distributed with poetry. We have them prominently in the readme too. One could argue that the interested user could install the development version, in that case I think we could clarify the situation.

In any case I am also fine with the solution proposed, in any case it's better than the current status. Happy for the team to decide as preferred.

[bachase]

I can see the benefit of including if we anticipate most users to test and work with them. The only mixed feeling I have is all those raw benchmark data results being inside the package, and then the mixing of scripts, python notebooks and what I think is earlier test code. Probably ok in the alpha stage but part of me feels its better to have ucc package released as polished as possible, and ok to keep the benchmarking code more messy for now.