Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding embed for AbstractStabilizer and Tableau #342

Closed
wants to merge 1 commit into from

Conversation

Fe-r-oz
Copy link
Contributor

@Fe-r-oz Fe-r-oz commented Aug 16, 2024

Adding embed for AbstractStabilizer

This PR implements embed for Stabilizer. This will be helpful in completing one of the task which is embed(..., sm) from #260!

In doing this, it address partially #151 as well.

At the moment, I have doctested them to see that embed is working correctly (both in pauli placement and phase).
...
If you want to submit an unfinished piece of work in order to get comments and discuss, please mark the pull request as a draft and ping the repository maintainer.

Please address only one topic or issue per pull request! Many small PRs are much easier to review and merge than one large PR.

Before merging, all changes and new functionality should be marked in the CHANGELOG file, but feel free to just leave your CHANGELOG notes in the PR description, to avoid merge conflicts with other requests modifying that file. The maintainer will add these CHANGELOG notes for you if you do so.

Before considering your pull request ready for review and merging make sure that all of the following are completed (please keep the clecklist as part of your PR):

  • The code is properly formatted and commented.
  • Substantial new functionality is documented within the docs.
  • All new functionality is tested.
  • All of the automated tests on github pass.
  • We recently started enforcing formatting checks. If formatting issues are reported in the new code you have written, please correct them. There will be plenty of old code that is flagged as we are slowly transitioning to enforced formatting. Please do not worry about or address older formatting issues -- keep your PR just focused on your planned contribution.

P.S. (some time later) With regard to Task 5, there was a minor formatting issue in test_throws that was introduced as the test_item changes (so the code is not that old), I hope that we can format it so that it conincides with formatting in all other tests. I just read the last point carefully now, I was skipping it as it was too long to read!

If possible, keep your git history not too wild (rebase and squash commits, keep commits small and semantically separated) so that review is easier.

@Fe-r-oz Fe-r-oz force-pushed the embed branch 2 times, most recently from 22ef6c5 to 667d748 Compare August 16, 2024 16:02
@Fe-r-oz
Copy link
Contributor Author

Fe-r-oz commented Aug 16, 2024

Also, there was formatting issue with test_throws.jl, only two .. when we have .... for space. Please see: https://github.com/QuantumSavory/QuantumClifford.jl/blob/master/test/test_throws.jl by opening it locally.

Fixed this as well while adding test throws for embed(..., Stabilizer). ALso, added tests in test_embed.

@Fe-r-oz Fe-r-oz marked this pull request as ready for review August 17, 2024 02:39
@Fe-r-oz Fe-r-oz changed the title Adding embed for AbstractStabilizer Adding embed for AbstractStabilizer and Tableau Aug 17, 2024
@Fe-r-oz
Copy link
Contributor Author

Fe-r-oz commented Aug 17, 2024

Added embed method for Tableau as well so Tableau with T ortab(destab)etc can be embedded.

@Krastanov
Copy link
Member

The QuantumInterface.embed is defined for operators (by padding the rest of the subsystems with identity operations). I am not sure what its meaning for states would be. I can imagine it meaning "pad the rest of the subsystems with completely mixed states" in which case this can be defined for MixedDestabilizer, but not for Tableau or Stabilizer.

I prefer to not commit to a new meaning for the embed function. Nonetheless, this functionality can be useful, but probably as an addition to hcat or as some new padding function. Overall though, lets wait for this to be necessary for something before implementing it.

@Krastanov Krastanov closed this Sep 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants