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

Fix a bug in the generator of DoubleExcitationPlus #298

Merged
merged 5 commits into from
Jun 1, 2022

Conversation

maliasadi
Copy link
Member

Context:
Fix a bug in applyGeneratorDoubleExcitationPlus.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 1, 2022

Hello. You may have forgotten to update the changelog!
Please edit .github/CHANGELOG.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 1, 2022

Test Report (C++) on Ubuntu

           1 files  ±    0             1 suites  ±0   1s ⏱️ ±0s
       862 tests ±    0         862 ✔️ ±    0  0 💤 ±0  0 ±0 
228 704 runs  +180  228 704 ✔️ +180  0 💤 ±0  0 ±0 

Results for commit b48d04a. ± Comparison against base commit a113c30.

♻️ This comment has been updated with latest results.

@AmintorDusko
Copy link
Contributor

Hi @maliasadi. Nice catch!
I wonder why the unit tests didn't get this bug.

@maliasadi
Copy link
Member Author

Hi @maliasadi. Nice catch! I wonder why the unit tests didn't get this bug.

Hmm! It required an improvement in generator tests to catch this bug.

@chaeyeunpark
Copy link
Contributor

Hi @maliasadi. Nice catch! I wonder why the unit tests didn't get this bug.

This bug does not appear when the number of qubits is 4 (the minimum number of wires required by DoubleExcitationPlus). As we did not have 4 qubit operators before, I just used the fixed number of qubits = 4 in generator test suits. I now update it and test up to 8 qubits.

@AmintorDusko AmintorDusko self-requested a review June 1, 2022 19:16
Copy link
Contributor

@AmintorDusko AmintorDusko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to approve. Once again, nice catch!

@codecov
Copy link

codecov bot commented Jun 1, 2022

Codecov Report

Merging #298 (b48d04a) into master (a113c30) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #298   +/-   ##
=======================================
  Coverage   99.42%   99.42%           
=======================================
  Files          30       30           
  Lines        3300     3300           
=======================================
  Hits         3281     3281           
  Misses         19       19           
Impacted Files Coverage Δ
pennylane_lightning/_version.py 100.00% <100.00%> (ø)
...src/gates/cpu_kernels/QChemGateImplementations.hpp 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 a113c30...b48d04a. Read the comment docs.

@maliasadi maliasadi merged commit e182d63 into master Jun 1, 2022
@maliasadi maliasadi deleted the fix_gen_dexcitationplus branch June 1, 2022 19:38
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.

3 participants