[unitaryhack] Added local complementation function to EGraph class

[smtsjhr]

Context for changes

• New features:

  • A method is added to the EGraph class to check "LC equivalence" of two graphs.

  • This method goes beyond the specifications given is issue 71, and also returns the Local Clifford operation which satisfies the equivalence. The desired output form of the clifford should be discussed. This pull request adds an optional string argument named clifford_form to the method, discussed in more detail below. Perhaps this option can be removed from the implementation once a single desired form is agreed upon

  • This method's call signature does not use the @staticmethod as suggested in comment here. I chose the original requested signature of issue 71 due to the Asymmetry of the returned Clifford when checking equivalence in two directions: graph1 --> graph2 compared to graph2 --> graph1. See more comments about this below.

  • A test file named 'test_is_lc_equivalent.py' is added in the directory './Tests/qubit_codes'

• Bug fixes:

  • None

• Improvements:

  • New functionality added and tests.

• Documentation changes:

  • No changes or additions to documentation for this new method have been made (yet).

Example usage and tests

This method checks if two graphs are LC equivalent. Here, "LC" means "Local Complementation / Local Clifford". When two graphs are LC equivalent they are related by a Local Clifford operation, and this method also returns this Local Clifford operation expressed in binary symplectic form.

The call signature of this method is:
graph1.is_lc_equivalent(graph2, clifford_form) --> (equiv, clifford)
Args::
Here graph1 and graph2 are EGraph objects.
clifford_form is a string (either 'global' or 'tensor') which specifies the output form of the clifford if the equivalence is True.
clifford_form is an optional argument with a default value of 'global'.
Returns:
The method returns a tuple of the form (equiv, clifford).
If the graphs are equivalent, equiv is True and clifford is the corresponding local clifford.
If the graphs are not equivalent, equiv is False and clifford is None.
When two graphs are equivalent the following applies:
If clifford_form = 'global', the returned clifford is a single 2nx2n binary matrix, where n is the number of nodes of the graphs.
If clifford_form = 'tensor', the returned clifford is given as a length n list of 2x2 binary matrices corresponding to the n (Hilbert space) tensor factors, where the k-th elemenet of the list is the tensor factor acting on the k-th node/qubit.

Note the following asymmetry if graph1 and graph2 are LC equivalent:
We can compare the graphs in two ways:
graph1 --> graph2 :: by calling graph1.is_lc_equivalent(graph2)
graph2 --> graph1 :: by calling graph2.is_lc_equivalent(graph1)
If the two graphs are LC equivalent then both calls return True equivalence, but the resulting cliffords may be distinct.
Therefore, we define test functions for both comparisons in order to test both cases.

Performance results justifying changes

• Not Applicable

Workflow actions and tests

TESTING METHODOLOGY:

The defined test functions run tests on various families of graphs known to be equivalent or not.
These include:

• Path Graph on 3 nodes <--> Complete Graph on 3 nodes (Equivalent)
• Star Graph on n nodes <--> Complete Graph on m nodes (Equivalent only when n=m)
• Empty Graph --> Empty Graph (currently assumed to be not equivalent, but we could opt to raise a ValueError)

When two graphs are equivalent, in order to verify the returned local clifford we perform the following algebraic check:
Let G1 and G2 be the adjacency matrices of the two LC equivalent graphs, and let the local clifford operation be given in block form as:
[A | B]
[C | D]
Then the following matrix block equation must hold True:
G2*(C*G1+D) == (A*G1+B)

Expected benefits and drawbacks

Expected benefits:

• New functionality to check LC equivalence between two graphs

Possible drawbacks:

• Test script may be lacking full scope. In particular, the method implementation does some error/exception handling, but the relevant tests may not be robust or lacking entirely. In general, broader tests could be added to test more diverse graphs and cases. As currently written, the test functions may also be redundant.

Related Github issues

This PR closes #71 which was marked for a UnitaryHack bounty.

Checklist and integration statements

• My Python and C++ codes follow this project's coding and commenting styles as indicated by existing files. Precisely, the changes conform to given black, docformatter and pylint configurations.

• I have performed a self-review of these changes, checked my code (including for codefactor compliance), and corrected misspellings to the best of my capacity. I have synced this branch with others as required.

• I have added context for corresponding changes in documentation and README.md as needed.

• I have added new workflow CI tests for corresponding changes, ensuring codecoverage is 95% or better, and these pass locally for me.

• I have updated CHANGELOG.md following the template. I recognize that the developers may revisit CHANGELOG.md and the versioning, and create a Special Release including my changes.

[smtsjhr]

Dear Devs,

This Pull Request may require some serious review due to my amateur coding. In particular, I attempted to write unit tests for the first time, so I feel a bit insecure about that. I hope you can bear with me! I still hope to complete the "Checklist and integration statements" outlined above.

My contributed code is heavily commented for the sake of review.

As this Pull Request is being made near the deadline for UnitaryHack, I do not demand nor expect to be eligible for potential bounties. Regardless, I am grateful for the opportunity to participate and happy to continue making revisions after the fact if possible.

Cheers,
Sumit

[codecov]

Codecov Report

Merging #89 (65e115e) into main (4b73a62) will increase coverage by 0.03%.
The diff coverage is 96.96%.

❗ Current head 65e115e differs from pull request most recent head a2af956. Consider uploading reports for the commit a2af956 to get more accurate results

@@            Coverage Diff             @@
##             main      #89      +/-   ##
==========================================
+ Coverage   96.19%   96.23%   +0.03%     
==========================================
  Files          36       36              
  Lines        2209     2308      +99     
==========================================
+ Hits         2125     2221      +96     
- Misses         84       87       +3     

Impacted Files	Coverage Δ
flamingpy/codes/graphs/egraph.py	95.28% <96.96%> (+1.80%)	⬆️

---

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 4b73a62...a2af956. Read the comment docs.

[soosub]

Awesome @smtsjhr! Thanks a lot for the contribution. I am not sure if we'll be able to get it in today, but I will check the rules to see if there is some slack for reviews.
In any case, I will ask @bestquark and either @ilan-tz or @nariman87 for review. I also approved the CI checks for this PR and it seems like it passes all the tests, good job 👍🏼 Can you also make sure it passes the formatting, linting, and CodeFactor checks? I think most of the current issues should be fixed if you run black (with option -l 100 to set the line length to 100) and docformatter on the relevant files.

Cheers!

[soosub]

@smtsjhr I just checked, and we don’t have to merge it before the UnitaryHack ends. To make sure you get the price, we simply have to mark your PR as accepted.
My proposal is as follows: today we will go over your PR today to see if it works and meets the required quality. If it does, we will mark it as accepted and you will get your price 💰 Then next week, we can do a proper review and correction to actually get it merged. The corrections can either be done by the devs or, if you want, by you. Just let us know 🤙🏼

[smtsjhr]

Awesome @smtsjhr! Thanks a lot for the contribution. I am not sure if we'll be able to get it in today, but I will check the rules to see if there is some slack for reviews. In any case, I will ask @bestquark and either @ilan-tz or @nariman87 for review. I also approved the CI checks for this PR and it seems like it passes all the tests, good job 👍🏼 Can you also make sure it passes the formatting, linting, and CodeFactor checks? I think most of the current issues should be fixed if you run black (with option -l 100 to set the line length to 100) and docformatter on the relevant files.

Cheers!

Thanks, @soosub ! I will follow through with the formatting, linting, and CodeFactor checks this weekend. I am more than happy to participate in further review and make changes as the team sees fit. This is my first major open source pull request, and I hope to follow best practices, so please excuse any friction from me end!

[bestquark]

Hey @smtsjhr, great work! Everything works as it should. I will review next week after all checks are successful. We have accepted your PR as the solution of this unitary hack challenge :)

[bestquark]

Great work!
Could you please run black -l 100 flamingpy/* to pass the formatting checks?
Also, could you fix the linting/codefactor issues? These are small issues such as lines being too long (such as strings) which you can check here (1) and here (2).
Thanks for your contribution, @smtsjhr :)

[smtsjhr]

Great work! Could you please run black -l 100 flamingpy/* to pass the formatting checks? Also, could you fix the linting/codefactor issues? These are small issues such as lines being too long (such as strings) which you can check here (1) and here (2). Thanks for your contribution, @smtsjhr :)

Hi @bestquark (CC: @soosub),

I have gone through my code to fix various formatting and linting issues. I ran black -l 100 locally, and also manually fixed issues with long comment line lengths and some other small fixes. I am now seeing "Failure" status for the github action, and am not sure what further actions I should take next.

[soosub]

Great work! Could you please run black -l 100 flamingpy/* to pass the formatting checks? Also, could you fix the linting/codefactor issues? These are small issues such as lines being too long (such as strings) which you can check here (1) and here (2). Thanks for your contribution, @smtsjhr :)

Hi @bestquark (CC: @soosub),

I have gone through my code to fix various formatting and linting issues. I ran black -l 100 locally, and also manually fixed issues with long comment line lengths and some other small fixes. I am now seeing "Failure" status for the github action, and am not sure what further actions I should take next.

Thanks! For black we recently included the test files, you can simply run black -l 100 flamingpy tests and that should fix it. This makes sure that it corrects all .py files in the flamingpy and tests folder.

Regarding the other issues you can usually see what went wrong by clicking on "details" (see figure below)

Note that for the CodeFactor checks you can ignore the "Method could be a function" warnings. We only use classes there to group the tests together so we can be more selective when running them. However, CF doesn't like that we use classes without actually using classes (e.g. make references to self and whatnot), hence the warnings.

The fail for linting is because there is a docstring missing somewhere.

[bestquark]

Dear @smtsjhr,
For the formatting, I forgot to mention that there are additional checks. These you should be able to pass by running the following lines:
black -l 100 tests flamingpy
docformatter --in-place flamingpy/*/*.py
docformatter --in-place tests/*/*.py
Could you try running these and check if the checks pass?
Thank you again for your great contribution!

EDIT: Taking a better look at CodeFactor, there are a few minimal issues to be solved, mainly, is_lc_equivalent is too complex. Could you also refactor some of these functionalities in private methods to avoid this?
For example, the code blocks under the lines

# Construct a binary system of equations that two graph adjacency matrices G and H must
...
# Puts a binary matrix into Row Reduced Echelon form modulo 2 up to a maximum number of
...
# Construct an array whose rows are basis vectors of the Null space of system_constraints M
...
# search through sums of pairs of basis vectors of the null space, and check if any satisfy
...

could each be made in a smaller private method (e.g. _construc_bin_system_of_eqs(self, ...), _reduced_echoleon_form(self, ...), etc.). Thanks a lot @smtsjhr 😄

[smtsjhr]

Dear @smtsjhr, For the formatting, I forgot to mention that there are additional checks. These you should be able to pass by running the following lines: black -l 100 tests flamingpy docformatter --in-place flamingpy/*/*.py docformatter --in-place tests/*/*.py Could you try running these and check if the checks pass? Thank you again for your great contribution!

EDIT: Taking a better look at CodeFactor, there are a few minimal issues to be solved, mainly, is_lc_equivalent is too complex. Could you also refactor some of these functionalities in private methods to avoid this? For example, the code blocks under the lines

# Construct a binary system of equations that two graph adjacency matrices G and H must
...
# Puts a binary matrix into Row Reduced Echelon form modulo 2 up to a maximum number of
...
# Construct an array whose rows are basis vectors of the Null space of system_constraints M
...
# search through sums of pairs of basis vectors of the null space, and check if any satisfy
...

could each be made in a smaller private method (e.g. _construc_bin_system_of_eqs(self, ...), _reduced_echoleon_form(self, ...), etc.). Thanks a lot @smtsjhr 😄

Hey @bestquark, thanks for the tips! I will run the formatting tests and do the refactoring as you mentioned.

Amusingly, my original implementation did have separate functions exactly how you suggest, but for some reason i bundled them all into one for pull request here! I will gladly make these changes this weekend.

With some more time I hope to make a technical review/blog about this algorithm and my implementation, which I can share here with any code reviewers if interested.

[soosub]

Sounds great @smtsjhr!

With some more time I hope to make a technical review/blog about this algorithm and my implementation, which I can share here with any code reviewers if interested.

Just letting you know: if you feel like it, you could also contribute to our tutorials page that we also keep on our repo 👀 We would happily collaborate on writing something up with you if you want.

[smtsjhr]

Hi Team!

Took me a bit to get to this refactoring but now my most recent commits reflect the following major changes::

The main method is_lc_equivalent() has been refactored to shorten its complexity. To achieve this, several "private" methods are introduced to the Egraph class. These include:
__system_constraints()
__RREform_mod2()
__nullspace_basis()
__search_nullspace()
__clifford_vec_to_tensors()
__clifford_vec_to_global()

The first 4 of these new private methods follow @bestquark 's previous suggestion, while I took my own liberty to introduce the last 2 methods which provide functionality to convert the representation of the local clifford into two different forms: 'tensor' or 'global'. Perhaps I should make these private function names more descriptive?

IMPLEMENTATION NOTE::
These private methods were defined using the __double_underscore prefix as opposed to the _single_underscore prefix. Due to my ignorance, I am not too sure what type would be the best formality here, so please advise if necessary.

TESTING NOTE::
As this refactoring was done without a test-driven approach, no unit tests were modified or introduced in the "test_is_lc_equivalent.py" file, which remains as it was before refactoring. This means only the main method is_lc_equivalent() is actually tested. However, the refactored code still passes all the existing tests. Please advise if further unit tests should be made for the new private methods. (I am thinking YES!)

Formatting/DocStrings NOTE::
I ran the following suggested commands locally and they seem to pass:
black -l 100 tests flamingpy
docformatter --in-place flamingpy/*/*.py
docformatter --in-place tests/*/*.py
Moreover, I think the docstrings I wrote for the main and private methods can be improved. I will try to study the existing conventions and edit the docstrings accordingly.

Of course, feel free to let me know of any desired changes and I will happily try my best to make them.

[bestquark]

Dear @smtsjhr,

Thank you for making these changes 😄. It seems to be still very few and minor issues with docformatter and code factor (which can be solved quite fast) that make two of the checks fail.

The first issue can be easily fix by running
docformatter --in-place flamingpy/*/*/*.py to 'docformat' the egraph file (since it is one directory deeper).

The second issue (CodeFactor) refers to a few functions (__system_constraints , __clifford_vec_to_tensors , __search_nullspace , __RREform_mod2 , __clifford_vec_to_global , test_pathgraph3_with_completegraph3_equivalent , test_completegraph_with_stargraph_equivalent, test_stargraph_with_completegraph_equivalent, test_completegraph3_with_pathgraph3_equivalent, test_emptygraph_with_emptygraph_assume_notequivalent, test_completeraph_with_stargraph_notequivalent, test_stargraph_with_completegraph_notequivalent, and test_emptygraph_with_emptygraph_assume_valueerror) being methods of classes that do not make use of any class attribute/methods (i.e self.something for the graph) or do not use a similar input (i.e. mode for the tests), therefore could be implemented as stand-alone-functions.

My recommendation to solve these second issues would be to take all the refactored methods that can be independent functions (the ones mentioned in the list) for testing lc equivalence and define them outside the egraph class. As for the test function, you could use the @pytest.mark.parametrize("mode", ["global", "tensor"]) just before defining TestLCEquivalent and leave inside this class all methods that make use of mode (so you don't need repeating this line for every method that uses mode). For the methods that have a different parametrization, you could move the outside TestLCEquivalent and test them independently.

With these two changes, I believe all checks should pass and we will be able to merge your PR.
Thanks again for your great contribution to FlamingPy :) @smtsjhr.

[smtsjhr]

Dear @smtsjhr,

Thank you for making these changes 😄. It seems to be still very few and minor issues with docformatter and code factor (which can be solved quite fast) that make two of the checks fail.

The first issue can be easily fix by running docformatter --in-place flamingpy/*/*/*.py to 'docformat' the egraph file (since it is one directory deeper).

The second issue (CodeFactor) refers to a few functions (__system_constraints , __clifford_vec_to_tensors , __search_nullspace , __RREform_mod2 , __clifford_vec_to_global , test_pathgraph3_with_completegraph3_equivalent , test_completegraph_with_stargraph_equivalent, test_stargraph_with_completegraph_equivalent, test_completegraph3_with_pathgraph3_equivalent, test_emptygraph_with_emptygraph_assume_notequivalent, test_completeraph_with_stargraph_notequivalent, test_stargraph_with_completegraph_notequivalent, and test_emptygraph_with_emptygraph_assume_valueerror) being methods of classes that do not make use of any class attribute/methods (i.e self.something for the graph) or do not use a similar input (i.e. mode for the tests), therefore could be implemented as stand-alone-functions.

My recommendation to solve these second issues would be to take all the refactored methods that can be independent functions (the ones mentioned in the list) for testing lc equivalence and define them outside the egraph class. As for the test function, you could use the @pytest.mark.parametrize("mode", ["global", "tensor"]) just before defining TestLCEquivalent and leave inside this class all methods that make use of mode (so you don't need repeating this line for every method that uses mode). For the methods that have a different parametrization, you could move the outside TestLCEquivalent and test them independently.

With these two changes, I believe all checks should pass and we will be able to merge your PR. Thanks again for your great contribution to FlamingPy :) @smtsjhr.

I have pushed the following changes:

• Successfully ran docformatter --in-place flamingpy/*/*/*.py

• In egraph.py, all "private" methods i had previously introduced are now moved to outside the Egraph class definition. Their names are no loner prefixed with any underscores.

• In test_is_lc_equivalent.py, the main test class is now parameterized for the two mode options: "global" and "tensors". This removed the redundancy from parametrizing the class test functions individually. This parameterization change allowed for the removal of two utility functions which were no longer relevant. NOTE that no test functions were moved to outside the class, and the test script still runs the same amount of tests as before this change: 70 Passing tests + 2 Skipped tests + (0 Failed tests)

Please do let me know if i missed anything!

**POST EDIT::
It looks like this push is failing the CodeFactor checks, consisting of 8 "Method could be a function" issues in the test file. I have been ignoring these as @soosub previously suggested here:

Note that for the CodeFactor checks you can ignore the "Method could be a function" warnings. We only use classes there to group the tests together so we can be more selective when running them. However, CF doesn't like that we use classes without actually using classes (e.g. make references to self and whatnot), hence the warnings.

I am assuming this is still okay. Otherwise, I think i understand how to structure the test script by removing the single test Class and defining its containing test methods individually. However, doing so seems to defeat the purpose of this last change where we mark.parametrize() the Class with mode variable instead of each individual test function. I am not sure whats the best practice here...

[nariman87]

Hi @smtsjhr and @bestquark,

Are the only remaining issues the minor conflict and syncing with the main? If yes and @smtsjhr allows it, I can directly work on the source branch to resolve them myself.

[bestquark]

Hi @nariman87, yes, this PR is ready and reviewed.

[nariman87]

Hi @nariman87, yes, this PR is ready and reviewed.

Thanks, @bestquark, however, best to first resolve the conflict and sync issues as I suggested.

[ilan-tz]

Actually @nariman87 @bestquark, I was going to have a quick look myself as well - let me finalize the review and we can take it from there.

[ilan-tz]

Excellent! This is a useful, interesting, and convenient addition that can be used to analyze the local equivalence of graph states. It's a product of a lot of work.

I have left some minor suggestions for names and docstrings. To finalize the PR, I think we should do the following:

• Place all new functions in a separate file in the utils' directory (say, linalg.py`).
  • is_lc_equivalent would then have to be modified very slightly, and it should be renamed to are_lc_equivalent.
  • The existing is_lc_equivalent in EGraph (the name can stay) should be a wrapper of are_lc_equivalent.
  • The name test_is_lc_equivalent should be renamed to something more general, like test_linalg.
• A small example of the functionality should be added to examples\graphstates.py.

[smtsjhr]

Excellent! This is a useful, interesting, and convenient addition that can be used to analyze the local equivalence of graph states. It's a product of a lot of work.

I have left some minor suggestions for names and docstrings. To finalize the PR, I think we should do the following:

• Place all new functions in a separate file in the utils' directory (say, linalg.py`).

  • is_lc_equivalent would then have to be modified very slightly, and it should be renamed to are_lc_equivalent.
  • The existing is_lc_equivalent in EGraph (the name can stay) should be a wrapper of are_lc_equivalent.
  • The name test_is_lc_equivalent should be renamed to something more general, like test_linalg.

• A small example of the functionality should be added to examples\graphstates.py.

Hi @ilan-tz and team,

I am happy to try to make these changes this weekend, if thats okay. I may circle back for some more clarification if needed. I am excited and determined to see this UnitaryHack contribution through! Thanks for your review and all of your suggestions.

[smtsjhr]

Excellent! This is a useful, interesting, and convenient addition that can be used to analyze the local equivalence of graph states. It's a product of a lot of work.

I have left some minor suggestions for names and docstrings. To finalize the PR, I think we should do the following:

• Place all new functions in a separate file in the utils' directory (say, linalg.py`).

  • is_lc_equivalent would then have to be modified very slightly, and it should be renamed to are_lc_equivalent.
  • The existing is_lc_equivalent in EGraph (the name can stay) should be a wrapper of are_lc_equivalent.
  • The name test_is_lc_equivalent should be renamed to something more general, like test_linalg.

• A small example of the functionality should be added to examples\graphstates.py.

Hey @ilan-tz.

I attempted to make all the changes you suggested, EXCEPT the last one for adding an example to examples\graphstates.py, which i still hope to do.

I am not sure if my way of "wrapping" the new utility function are_lc_equivalent in the is_lc_equivalent method of the EGraph class meets your suggestions, so please feel free to make or suggest any more changes.

The contents of the renamed test file test_linalg.py have not been modified, so it still is only testing the is_lc_equivalent method of the EGraph class directly. No tests have been modified or added to directly test the functions of linalg.py. Perhaps this is something we should do?

The other commits I recently made were minor formatting changes to clean up the code.

Again, an example of the functionality still needs to be added to examples\graphstates.py. If you have any specific suggestions of what you would like to see there, I can try to make that happen.

[ilan-tz]

Excellent! This is a useful, interesting, and convenient addition that can be used to analyze the local equivalence of graph states. It's a product of a lot of work.
I have left some minor suggestions for names and docstrings. To finalize the PR, I think we should do the following:

• Place all new functions in a separate file in the utils' directory (say, linalg.py`).

  • is_lc_equivalent would then have to be modified very slightly, and it should be renamed to are_lc_equivalent.
  • The existing is_lc_equivalent in EGraph (the name can stay) should be a wrapper of are_lc_equivalent.
  • The name test_is_lc_equivalent should be renamed to something more general, like test_linalg.

• A small example of the functionality should be added to examples\graphstates.py.

Hey @ilan-tz.

I attempted to make all the changes you suggested, EXCEPT the last one for adding an example to examples\graphstates.py, which i still hope to do.

I am not sure if my way of "wrapping" the new utility function are_lc_equivalent in the is_lc_equivalent method of the EGraph class meets your suggestions, so please feel free to make or suggest any more changes.

The contents of the renamed test file test_linalg.py have not been modified, so it still is only testing the is_lc_equivalent method of the EGraph class directly. No tests have been modified or added to directly test the functions of linalg.py. Perhaps this is something we should do?

The other commits I recently made were minor formatting changes to clean up the code.

Again, an example of the functionality still needs to be added to examples\graphstates.py. If you have any specific suggestions of what you would like to see there, I can try to make that happen.

Hi @smtsjhr ! Thanks for your quick turnaround on this. On first glance the wrapping looks exactly as I intended. I'd say we don't have to add more linalg tests to this PR. For the example, I propose the following:

• From utils.graph_states import star_graph, complete_graph, linear_cluster, ring_graph.
• Confirm that star_graph(3), complete_graph(3), linear_cluster(3) and ring_graph(3) are all LC-equivalent.

This is a quick way to verify the validity of the function and show users that we have a small existing database of graph states. Please let me know if you're able to put this example inl if so, once it's ready I'll be happy to re-review.

Thanks again for your work!

[smtsjhr]

Hi @ilan-tz,

I added a new example file: .../flamingpy/examples/lc_equivalence.py

I feel my example code is a bit redundant, and considered using some kind of loop to log all the different graph comparisons.

Also, I opted to include some global parameters nodes and clifford_form to allow the user to easily change them.

Previously, I think you suggested adding a new example for testing LC equivalence to the already existing example file .../flamingpy/examples/graphstates.py. I didn't do that yet, and just created a new file for now. I tried to add my example to that script, but for some reason I am not able to log the scripts output when I try. I am not sure why, but think the output is being blocked when trying to show the plots from the pre-existing code).

I will wait for further review to make more changes... Thanks!

[ilan-tz]

@smtsjhr Looks great! Just a few tiny suggestions made -- please also delete .DS_Store & sync with the latest main, and then this is ready to merge (if you are done with changes on your end).

[smtsjhr]

🚨 🚨 🚨
Hey @ilan-tz !

I made a big mistake and need advice to fix!

When trying to sync my fork to the latest main I accidentally "force-pushed", which discarded 45 commits from my original fork. This seemed to have "merged" the forks to the latest main, and closed this pull request (without my intention).

What is the best action I should take to revert this merge on my fork, and preserve my commits?

OPTION 1::
I could easily add the missing code and files manually, but do you suggest a better way?

OPTION 2::
Perhaps I can use the following git command to revert changes:
git reset --merge a2af956
where a2af956 is the last commit hash before the force-push.

OPTION 3::
My local repo of my fork still has my commits. Should I force git push to overwrite the remote repo in order to restore it?

OPTION 4::
???

Sorry to spoil this so close to completion. I am nervous now of making things worse, so will wait for any tips. Thanks! 🙏

[nariman87]

Hi @smtsjhr,

In this case and after the mistakes and changes above, XanaduAI:main flamingpy is, of course, mostly unaffected (by design only mistakes from admins and some other users can harm the project -- we may change permissions as needed). The only downside is that some lines of unnecessary history are now added to the project forever, such as this PR or a history line saying smtsjhr merged 0 commits into [XanaduAI:main] from [smtsjhr:main]. No problem for now, but we appreciate it if you avoid similar occurrences while making valuable contributions.

Now, what I suggest in your situation (and indeed you are free to do as you wish with smtsjhr's files and branches):

• Make a local copy of all files in your local branch that you mentioned has all your latest commits.
• While almost everything in git can be reverted and new remotes can be added as needed, my suggestion is to create a new branch, and copy all the files from your existing local branch to it. Then push this new branch to a remote.
• Next, you must create a new PR to XanaduAI:main flamingpy again and feel free to hint in the new PR that it's a follow-up to this one, that was accidentally wiped and merged.

[smtsjhr]

Hi @nariman87 (CC @ilan-tz ),

Hoping I correctly recovered from my accidental merge. Feel free to let me know if any more action is needed on my end.

Please refer to Pull #117 for followup on this resubmitted pull request.