Fix an edge case in Sabre's release valve

[Cryoris]

Summary

Fixes #13081.

Details and comments

If a Sabre trial does not find a set of Swaps to route nodes, the "release valve" adds Swaps to route the two-qubit gate in between the the closest two qubits. In rare cases, this leads to more than one gate being routable, which was not handled correctly previously.

As I understand it this should only ever occur if the release-valve adds a single swap and we have a scenario like

A - B - A - B  --apply swap--> A - A - B - B

If more than one swap gate is applied it should not be possible that two gates are routable, so the code now only checks for a second routable gate if we insert a single swap. If someone can find a counterexample we can change the code to check for any routable gate after the release valve 🙂

[qiskit-bot]

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

• @Qiskit/terra-core
• @kevinhartman
• @mtreinish

[kevinhartman]

I can't comment above this line, but is there any reason we shouldn't use a SmallVec for routable_nodes itself?

And, should we increase the capacity of the pre-allocation to 3 now on line 573?

[jakelishman]

We shouldn't necessarily, because the release valve should only be taken rarely, and we don't want to overallocate the happy path, and we should still only be routing two nodes in the new case.

[coveralls]

Pull Request Test Coverage Report for Build 10795320615

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 13 of 26 (50.0%) changed or added relevant lines in 2 files are covered.
• 14 unchanged lines in 2 files lost coverage.
• Overall coverage decreased (-0.02%) to 89.142%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
crates/accelerate/src/sabre/route.rs	9	22	40.91%

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/lex.rs	2	92.98%
crates/qasm2/src/parse.rs	12	97.15%

Totals
Change from base Build 10794993036:	-0.02%
Covered Lines:	73023
Relevant Lines:	81918

---

💛 - Coveralls

[Cryoris]

With #13119 merged, CI should pass now I think 🙂

[mtreinish]

This LGTM, I have a small inline nit comment on code style but it's not significant or a blocker. I'm running the 1000q QV reproducer I put in the original issue to confirm it's fixed once I confirm it's fixed I'll add this to the merge queue.

[mtreinish]

Couldn't this just be a single filter map. Something like:

Suggested change

	.filter_map(|&swap_qubit| self.front_layer.qubits()[swap_qubit.index()])
	// remove the closest_node, which we know we already routed
	.filter(|(node_index, _other_qubit)| *node_index != closest_node)
	.map(|(_node_index, other_qubit)| other_qubit);
	.filter_map(|&swap_qubit| {
	self.front_layer.qubits()[swap_qubit.index()]).map(|(node_index, other_qubit)| {
	// remove the closest_node, which we know we already routed
	if node_index != closest_node {
	Some(other_qubit)
	} else {
	None
	}
	})
	});

Or even:

Suggested change

	.filter_map(|&swap_qubit| self.front_layer.qubits()[swap_qubit.index()])
	// remove the closest_node, which we know we already routed
	.filter(|(node_index, _other_qubit)| *node_index != closest_node)
	.map(|(_node_index, other_qubit)| other_qubit);
	.filter_map(|&swap_qubit| self.front_layer.qubits()[swap_qubit.index()])
	// remove the closest_node, which we know we already routed
	.filter_map(|(node_index, other_qubit)| if *node_index != closest_node {
	Some(other_qubit)
	} else {
	None
	})

Not that this makes a big difference. I'm surprised clippy isn't complaining about the filter followed by a map though.

[jakelishman]

if you're really going hard on fancy methods, you can even do (*node_index != closest_node).then_some(other_qubit) haha.

[mtreinish]

The 1000 qubit QV circuit reproducer from #13081 works with this applied so I'm good to merge this.