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

Add IsingXY #303

Merged
merged 19 commits into from
Jun 14, 2022
Merged

Add IsingXY #303

merged 19 commits into from
Jun 14, 2022

Conversation

multiphaseCFD
Copy link
Member

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
    tests 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.

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

  • Add a new entry to the .github/CHANGELOG.md file, summarizing the
    change, and including a link back to the PR.

  • Ensure that code is properly formatted by running make format.

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


Context:

Description of the Change:

Benefits:

Possible Drawbacks:

Macbook air M1 doesn't support large memory allocation in the tests

Related GitHub Issues:

@github-actions
Copy link
Contributor

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.

@chaeyeunpark
Copy link
Contributor

Yes, there is an issue due to my poorly written workflow 😞 . You can just push an empty commit by

git commit --allow-empty -m 'Trigger CI'
git push

to rerun all workflows.

@multiphaseCFD
Copy link
Member Author

Yes, there is an issue due to my poorly written workflow 😞 . You can just push an empty commit by

git commit --allow-empty -m 'Trigger CI'
git push

to rerun all workflows.

Done!

@github-actions
Copy link
Contributor

github-actions bot commented Jun 14, 2022

Test Report (C++) on Ubuntu

           1 files  ±  0             1 suites  ±0   2s ⏱️ -1s
       912 tests +28         912 ✔️ +28  0 💤 ±0  0 ±0 
229 195 runs  +49  229 195 ✔️ +49  0 💤 ±0  0 ±0 

Results for commit 8d9f709. ± Comparison against base commit e4bca20.

♻️ This comment has been updated with latest results.

@codecov
Copy link

codecov bot commented Jun 14, 2022

Codecov Report

Merging #303 (8d9f709) into master (e4bca20) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #303   +/-   ##
=======================================
  Coverage   99.44%   99.45%           
=======================================
  Files          36       36           
  Lines        3605     3656   +51     
=======================================
+ Hits         3585     3636   +51     
  Misses         20       20           
Impacted Files Coverage Δ
...ennylane_lightning/src/gates/OpToMemberFuncPtr.hpp 100.00% <ø> (ø)
pennylane_lightning/_version.py 100.00% <100.00%> (ø)
...ng/src/gates/cpu_kernels/GateImplementationsLM.hpp 100.00% <100.00%> (ø)
...ng/src/gates/cpu_kernels/GateImplementationsPI.hpp 100.00% <100.00%> (ø)
pennylane_lightning/src/simulator/KernelMap.cpp 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 e4bca20...8d9f709. Read the comment docs.

@chaeyeunpark
Copy link
Contributor

It seems like all tests pass besides Format (C++). After installing clang-format-12, you may just run make format in the root of source code directory.

@multiphaseCFD
Copy link
Member Author

clang-format-12

Thanks! I failed to install clang-format on M1 and I tried to use black to make better format. Will try to install clang-format again and get back to you.

@AmintorDusko
Copy link
Contributor

AmintorDusko commented Jun 14, 2022

Hi @multiphaseCFD. I think you need to add .DS_store to the .gitignore file.
You may need to remove the files already committed.

Copy link
Contributor

@chaeyeunpark chaeyeunpark left a comment

Choose a reason for hiding this comment

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

I have one specific question here. I also would like to ask you benchmark two implementations. You can run make gbenchmark and follow the instruction in https://github.com/PennyLaneAI/pennylane-lightning/blob/master/pennylane_lightning/src/benchmarks/README.md#benchmarksbench_kernels. Note that you can also filter the benchmarks to run. For more information, you may run ./BuildGBench/benchmarks/bench_kernels --help

ComplexPrecisionT{0.089653238654, 0.221581339836},
ComplexPrecisionT{0.217892318964, 0.291261285543},
ComplexPrecisionT{0.292993247509, 0.186570793390},
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the result from default.qubit?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is this the result from default.qubit?

Yes

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.

Nice work, so far!

@multiphaseCFD
Copy link
Member Author

I have one specific question here. I also would like to ask you benchmark two implementations. You can run make gbenchmark and follow the instruction in https://github.com/PennyLaneAI/pennylane-lightning/blob/master/pennylane_lightning/src/benchmarks/README.md#benchmarksbench_kernels. Note that you can also filter the benchmarks to run. For more information, you may run ./BuildGBench/benchmarks/bench_kernels --help

Screen Shot 2022-06-14 at 2 37 03 PM

@chaeyeunpark
Copy link
Contributor

Thanks @multiphaseCFD for a nice job! Still, I think you may need to implement a generator for a backward path. When a gate is a one-paramter Lie group, given by $e^{-iG\theta}$, we call $G$ as the generator. Could you try this too? Please let me know when there is any question.

Copy link
Member

@mlxd mlxd left a comment

Choose a reason for hiding this comment

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

Nice work @multiphaseCFD !
I'll approve. Once the others approve too we can merge this in.
Also, agreed with @chaeyeunpark for the backwards pass generator,

@multiphaseCFD
Copy link
Member Author

Thanks @multiphaseCFD for a nice job! Still, I think you may need to implement a generator for a backward path. When a gate is a one-paramter Lie group, given by e−iGθ, we call G as the generator. Could you try this too? Please let me know when there is any question.

Sure! I will try this after I finalized current PR. As @mlxd told me, what I have to do for this PR is just implementing IsingXY gate. Is it fine with you @chaeyeunpark ?

@multiphaseCFD
Copy link
Member Author

Nice work @multiphaseCFD ! I'll approve. Once the others approve too we can merge this in. Also, agreed with @chaeyeunpark for the backwards pass generator,

Thanks @mlxd !

@chaeyeunpark
Copy link
Contributor

Sure! Looking forward to seeing the generator for IsingXY in a subsequent PR. I am also gonna approve this.

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.

Nice work! Thanks for that!

@multiphaseCFD
Copy link
Member Author

Sure! Looking forward to seeing the generator for IsingXY in a subsequent PR. I am also gonna approve this.

Sure! I've put it into my tasks.

@multiphaseCFD
Copy link
Member Author

Nice work! Thanks for that!

Thanks!

@multiphaseCFD multiphaseCFD merged commit a6f4c99 into master Jun 14, 2022
@multiphaseCFD multiphaseCFD deleted the origin/add_isingxy branch June 14, 2022 20:13
@mlxd mlxd mentioned this pull request Jun 15, 2022
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.

4 participants