Pr swap based transpiler

[mauriceweber]

Before submitting

Please complete the following checklist when submitting a PR:

• All new features must include a unit test.
  If you've fixed a bug or added code that should be tested, add a test to the
  test directory!

• All new functions and code must be clearly commented and documented.
  If you do make documentation changes, make sure that the docs build and
  render correctly by running make docs.

• Ensure that the test suite passes, by running make test.

• Add a new entry to the doc/releases/changelog-dev.md file, summarizing the
  change, and including a link back to the PR.

• The PennyLane source code conforms to
  PEP8 standards.
  We check all of our code against Pylint.
  To lint modified files, simply pip install pylint, and then
  run pylint pennylane/path/to/file.py.

When all the above are checked, delete everything above the dashed
line and fill in the pull request template.

---

Context:
Add a swap-based router transform

Description of the Change:
new module transpile.py in pennylane.transforms; the transpile function takes as input a tape and hardware connectivity graph, and compiles the tape to ensure that it can be executed on the corresponding hardware.

Related GitHub Issues:
#2115

[josh146]

Thanks @mauriceweber --- I skimmed through quickly, very thorough documentation! I'll leave a review shortly :)

[codecov]

Codecov Report

Merging #2118 (208d45e) into master (0814da9) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2118   +/-   ##
=======================================
  Coverage   99.42%   99.42%           
=======================================
  Files         243      244    +1     
  Lines       18338    18399   +61     
=======================================
+ Hits        18232    18293   +61     
  Misses        106      106           

Impacted Files	Coverage Δ
pennylane/transforms/__init__.py	100.00% <100.00%> (ø)
pennylane/transforms/transpile.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0814da9...208d45e. Read the comment docs.

[josh146]

@mauriceweber thank you, this is a great implementation! I went through and left some comments, mostly relating to documentation and testing :)

For the testing, Codecov raised some places where parts of the logic is not being tested, so it might be good to add some additional unit tests, checking that this works for various circuit edge cases (e.g., for circuits that return qml.probs()).

A final test that could be very nice to have: a test that ensures that after a circuit is transpiled, it remains differentiable. This is a good 'sanity check' test, since it can be very easy to accidentally break the differentiability when manipulating operations!

[josh146]

Hey @mauriceweber, I just wanted to check in here and see on the status of this PR :) Was this something you might have time to complete?

[mauriceweber]

Hi @josh146, yep I am planning to finish this still! The main thing left to do I think is to improve the functionality of the _adjust_mmt_indices function -- it currently doesn't support expectation values of tensor products and Hamiltonians. I will look into how we can achieve this:)

[josh146]

Great to hear @mauriceweber!

The main thing left to do I think is to improve the functionality of the _adjust_mmt_indices function -- it currently doesn't support expectation values of tensor products and Hamiltonians. I will look into how we can achieve this:)

This is completely up to you, I might suggest not including this in the first version of this functionality, as the Tensor class and the Hamiltonian class are likely going to be 'merged' into the main Operator class going forward!

If it is a quick and easy change, though, that would be fine. Otherwise, the following is also okay:

• We can open an issue detailing the need to be able to easily map the wires of tensor product/Hamiltonian objects

• In this PR, we add a .. warning:: box near the top of the docstring, informing readers that this function does not yet work for tensors/Hamiltonians.

• Within the code, we raise an error (for now) in those two cases.

[mauriceweber]

I couldn't find an easy fix for that, so let us not include it in the first version.

In this PR, we add a .. warning:: box near the top of the docstring, informing readers that this function does not yet work for tensors/Hamiltonians.

This is already included:

pennylane/pennylane/transforms/transpile.py

Lines 21 to 24 in 2746d6d

	.. warning::
	This transform does not yet support measurements of Hamiltonians or tensor products of observables. If a circuit
	is passed which contains these types of measurements, a NotImplementedError will be raised.

Within the code, we raise an error (for now) in those two cases.

This is also implemented already (both in the _adjust_mmt_indices function and in the transpile function itself):

pennylane/pennylane/transforms/transpile.py

Lines 172 to 176 in aab82fa

	if isinstance(_m.obs, (Hamiltonian, Tensor)):
	# tensor products of observables
	raise NotImplementedError(
	"Measuring expectation values of tensor products or Hamiltonians is not yet supported"
	)

pennylane/pennylane/transforms/transpile.py

Lines 92 to 95 in aab82fa

	if any(isinstance(m.obs, (Hamiltonian, Tensor)) for m in tape.measurements):
	raise NotImplementedError(
	"Measuring expectation values of tensor products or Hamiltonians is not yet supported"
	)

[josh146]

My mistake @mauriceweber, apologies!

I'll give it a final review, I just noticed that the tests aren't currently running due to the following issue:

 tests/transforms/test_transpile.py:6: in <module>
    import regex as re
E   ModuleNotFoundError: No module named 'regex'

I think you can remove this import, and simply write the test as follows:

err_msg = "Measuring expectation values of tensor products or Hamiltonians is not yet supported"
with pytest.raises(NotImplementedError, match=err_msg):
    transpiled_qnode()

[mauriceweber]

cool, thanks @josh146 !:)

I think you can remove this import, and simply write the test as follows:

Ah I see, I removed all the regex dependencies now, so that the tests should go through (had to remove some special characters from one error message)

[mauriceweber]

Hi @josh146 , I can see that some changes were requested but can't find what these are. Can you point me to the place where I can see the requested changes?

And also, it seems that the tests stalled. Do we need to retrigger them?

[josh146]

I can see that some changes were requested but can't find what these are. Can you point me to the place where I can see the requested changes?

Hey @mauriceweber, I think this is from my old review - let me clear that for you :) I will find some time in the next two days or so to go through and give it a fresh review!

And also, it seems that the tests stalled. Do we need to retrigger them?

That is very odd 🤔 I'm not sure why that has happened, I will need to investigate that.

[josh146]

@mauriceweber fantastic work! This looks almost ready for merging now 🙂

Once the __main__ block is removed, and the other suggestions addressed, let me know and I can merge this in 💯

[josh146]

@mauriceweber I don't think this error can ever be raised, since such cases should be caught by the exception on line 94? In which case, this can be removed :)

[mauriceweber]

oh yep, that won't be raised -- I removed it:)

[josh146]

While convenient for testing and prototyping, this section should be removed prior to merging!

[mauriceweber]

oh totally, I forgot to remove it after testing!

[josh146]

Thanks for this awesome feature @mauriceweber!

[mauriceweber]

thanks for the support and the code reviews @josh146 !:)