Add .index() and new() method to Qubit and Clbit

[mtreinish]

Summary

The Qubit and Clbit rust newtypes are a u32 that are used to track the Qubits or Clbits in a circuit. The u32 value represents the index of the Qubit or Clbit in the circuit and are often used as indices in Vecs or other structures where the index must be a usize. Accordingly there are a lot of times we need to convert a Qubit or Clbit object into a usize and previously the only way to do that was use .0 as usize on a given qubit object. To improve the ergonomics of that pattern this commit adds two new methods for converting from and to a usize new() and index() respectively. These are modeled after what petgraph does with it's NodeIndex type which is a similar pattern (although NodeIndex is generic over the inner type, although it defaults to u32).

Details and comments

[qiskit-bot]

One or more of the following people are relevant to this code:

• @Qiskit/terra-core

[coveralls]

Pull Request Test Coverage Report for Build 11242635350

Details

• 147 of 152 (96.71%) changed or added relevant lines in 14 files are covered.
• 8 unchanged lines in 4 files lost coverage.
• Overall coverage increased (+0.01%) to 88.885%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
crates/accelerate/src/synthesis/multi_controlled/mcmt.rs	2	3	66.67%
crates/accelerate/src/synthesis/clifford/greedy_synthesis.rs	18	20	90.0%
crates/circuit/src/lib.rs	18	20	90.0%

Files with Coverage Reduction	New Missed Lines	%
crates/circuit/src/dag_circuit.rs	1	88.41%
crates/qasm2/src/expr.rs	1	94.02%
qiskit/synthesis/two_qubit/xx_decompose/decomposer.py	1	95.45%
crates/qasm2/src/lex.rs	5	91.98%

Totals
Change from base Build 11241264267:	0.01%
Covered Lines:	74758
Relevant Lines:	84106

---

💛 - Coveralls

[eliarbel]

This is nice. A comment though: with both .index() and ::new() we can now make BitType private in Qubit and Clbit. But it's much nicer to write e.g. Qubit(0) than Qubit::new(0). So maybe we only need .index()?

[mtreinish]

This is nice. A comment though: with both .index() and ::new() we can now make BitType private in Qubit and Clbit. But it's much nicer to write e.g. Qubit(0) than Qubit::new(0). So maybe we only need .index()?

The problem is that as the inner type is BitType we can't pass a usize like Qubit(0usize), Qubit(0) works fine because the type inference knows 0 is a u32 in that context. But if you were to do something like: Qubit(qubits_vec.len() + 1) that would be a type mismatch I added the new() method so we could work with usize directly without needing to manually cast anything. That being said I didn't go through and update Qubit construction to use new() where applicable. I'll do that now.

[kevinhartman]

It's certainly a nice improvement for us to be rid of the casting peppered everywhere!

My main suggestion would be to add an assertion to {Qu|Cl}bit::new that the index fits, which would make things more sound than what we have today.

I'm in favor of keeping the BitType field public, esp. with the new constructor, since users really need to know that whatever they're stuffing into a qubit / clbit must fit within a u32. Can you add an explicit warning to the new methods that the usize needs to fit within a BitType?

[kevinhartman]

Thanks for the changes! This is looking good from my perspective now.