Add back qiskit.transpiler.passes.synthesis.graysynth module

[mtreinish]

Summary

This commit reverts a breaking change made in #8568, in that PR the graysynth module was moved to the qiskit.synthesis package and the cnot_synth function was also renamed as part of the move. However, this change would break any existing users of the module without any warning. To fix this for the 0.23.0 release this commit adds back the graysynth module and just exports the contents of the module in its new location. Additionally, a small wrapper function is added so that the legacy cnot_synth function name can continue to be used for the time being. In the 0.24.0 cycle we can deprecate the old module and remove them after we've giving users a warning about the change.

Details and comments

[qiskit-bot]

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the the following people are requested to review this:

• @Qiskit/terra-core

[kdk]

Looks reasonable to me. What do you think about reinstating the test for these functions?

[mtreinish]

I think the code is minimal enough here that we don't really need to worry about test coverage. All this is doing is redirecting calls from the old module to the new module and there is only one source of code so we'd essentially be testing python and not our code. That being said apparently this is causing a import cycle in the unit tests (which my manual tests didn't catch). I'll have to use a lazy import to fix this

[mtreinish]

I've changed my mind on the testing it might be effectively one line of code but I made a typo in this so I'll add a test case that exercises the function.

[coveralls]

Pull Request Test Coverage Report for Build 4000870400

• 10 of 10 (100.0%) changed or added relevant lines in 2 files are covered.
• 3 unchanged lines in 2 files lost coverage.
• Overall coverage increased (+0.03%) to 84.927%

---

Files with Coverage Reduction	New Missed Lines	%
src/vf2_layout.rs	1	86.44%
src/sabre_swap/layer.rs	2	98.95%

Totals
Change from base Build 3999824162:	0.03%
Covered Lines:	66719
Relevant Lines:	78560

---

💛 - Coveralls

[ShellyGarion]

Note that both gray_synth and cnot_synth have never appeared in the Qiskit documentation.

[alexanderivrii]

@mtreinish, thanks for taking care of this. Out of curiosity, did someone complain about the missing functionality? (Shelly and I were under an impression that if something is not mentioned anywhere in the documentation, it can probably be moved or renamed at will).

On a related note, we have also renamed some of the already existing (but not documented) clifford synthesis functions -- can this be a problem?

[mtreinish]

Note that both gray_synth and cnot_synth have never appeared in the Qiskit documentation.

IMO, this was mostly just an oversight. The module was added in the 0.9.0 release (back then we weren't as careful about the documentation) and the feature was documented as a new feature in the release notes at the time. For cases like this where it's not explicitly marked as internal or private (either with a leading _ or in the docstring) we should err on the side of caution and support the legacy name so if there are existing users they can migrate. I agree with the new location and name make it clearer, but it's very little effort on our part to add the redirects and do this and will avoid users complaining about things breaking for them.

@mtreinish, thanks for taking care of this. Out of curiosity, did someone complain about the missing functionality? (Shelly and I were under an impression that if something is not mentioned anywhere in the documentation, it can probably be moved or renamed at will).

Nobody complained about it afaik (although rc1 usage is typically pretty low so people might not have seen this) but I caught this going through the release notes and it was documented as a breaking change and I couldn't come up with a justification for why it needed to be breaking.

On a related note, we have also renamed some of the already existing (but not documented) clifford synthesis functions -- can this be a problem?

It could be if we weren't explicitly saying they were private or internal it might be best to add backwards compat wrappers to support the old names too.