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.

Sign up for GitHub

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

Jump to bottom

Add C++ unit tests for Exact Tensor Network backends #998

Merged
merged 32 commits into from
Nov 28, 2024
Merged

Add C++ unit tests for Exact Tensor Network backends #998

merged 32 commits into from
Nov 28, 2024

Conversation

LuisAlfredoNu
Copy link
Contributor

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.

  • 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:
Lightning.tensor device adding TN backend

Description of the Change:

  • Unit tests for gate support
  • Unit tests for measurements

Benefits:

Possible Drawbacks:

Related GitHub Issues:

[sc-77835]

@github-actions 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.

@maliasadi maliasadi added ci:build_wheels Activate wheel building. urgent Mark a pull request as high priority ci:use-gpu-runner Enable usage of GPU runner for this Pull Request labels Nov 18, 2024
@maliasadi maliasadi marked this pull request as ready for review November 18, 2024 15:46
@maliasadi maliasadi removed the urgent Mark a pull request as high priority label Nov 18, 2024
@codecov Codecov
Copy link

codecov bot commented Nov 18, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.93%. Comparing base (8b75653) to head (de2855e).
Report is 2 commits behind head on add_exatn_cpp.

Additional details and impacted files 
@@                Coverage Diff                 @@
##           add_exatn_cpp     #998       +/-   ##
==================================================
+ Coverage          39.15%   88.93%   +49.78%     
==================================================
  Files                 43      108       +65     
  Lines               2955    16524    +13569     
==================================================
+ Hits                1157    14696    +13539     
- Misses              1798     1828       +30

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

AmintorDusko
AmintorDusko reviewed Nov 19, 2024
View reviewed changes
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.

Few comments, but overall good.

pennylane_lightning/core/src/simulators/lightning_tensor/tncuda/CMakeLists.txt Outdated Show resolved Hide resolved
pennylane_lightning/core/src/simulators/lightning_tensor/tncuda/gates/tests/CMakeLists.txt Outdated Show resolved Hide resolved
pennylane_lightning/core/src/simulators/lightning_tensor/tncuda/gates/tests/CMakeLists.txt Outdated Show resolved Hide resolved
...ightning/core/src/simulators/lightning_tensor/tncuda/gates/tests/Test_ExaTNCuda_NonParam.cpp Outdated Show resolved Hide resolved
...lane_lightning/core/src/simulators/lightning_tensor/tncuda/measurements/tests/CMakeLists.txt Outdated Show resolved Hide resolved
maliasadi
maliasadi requested changes Nov 19, 2024
View reviewed changes
Copy link
Member

@maliasadi maliasadi 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 @LuisAlfredoNu! Just a few suggestions:

  • Update changelog,
  • Merge ETN and MPS C++ unit tests, and
  • Include your PL script as code comments to C++ tests files

multiphaseCFD
multiphaseCFD reviewed Nov 27, 2024
View reviewed changes
pennylane_lightning/core/src/simulators/lightning_tensor/tncuda/utils/tn/TestHelpersTNCuda.hpp

template <typename TNDevice_T>
inline void
tn_state_append_mps_final_state(std::unique_ptr<TNDevice_T> const &tn_state) {
Copy link
Member

Choose a reason for hiding this comment

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

Why not applying this method across all the tests, instead of measurements only?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are applying this method on measurements and gates. Would you like to use this method on observables test too?

LuisAlfredoNu
LuisAlfredoNu commented Nov 27, 2024
View reviewed changes
pennylane_lightning/core/src/simulators/lightning_tensor/tncuda/gates/tests/Test_TNCuda_MPO.cpp
Comment on lines +38 to +39
TEMPLATE_LIST_TEST_CASE("TNCuda::applyMPO::2+_wires", "[TNCuda_Nonparam]",
TestMPSBackends) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TypeList only with MPS backends

multiphaseCFD
multiphaseCFD reviewed Nov 28, 2024
View reviewed changes
...e_lightning/core/src/simulators/lightning_tensor/tncuda/gates/tests/Test_TNCuda_NonParam.cpp Outdated

using TNDevice_T = TestType;
using cp_t = typename TNDevice_T::ComplexT;
// using Precision_T = typename TNDevice_T::PrecisionT;
Copy link
Member

Choose a reason for hiding this comment

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

Please remove this commented line. Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

multiphaseCFD
multiphaseCFD reviewed Nov 28, 2024
View reviewed changes
Copy link
Member

@multiphaseCFD multiphaseCFD left a comment

Choose a reason for hiding this comment

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

Thanks @LuisAlfredoNu . One final comment and happy to approve after resolving it.

multiphaseCFD
multiphaseCFD approved these changes Nov 28, 2024
View reviewed changes
Copy link
Member

@multiphaseCFD multiphaseCFD left a comment

Choose a reason for hiding this comment

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

Thanks @LuisAlfredoNu ! Nice work! Happy to approve.

@LuisAlfredoNu LuisAlfredoNu merged commit c32ca00 into add_exatn_cpp Nov 28, 2024
33 of 34 checks passed
@LuisAlfredoNu LuisAlfredoNu deleted the add_exatn_cpp_test branch November 28, 2024 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AmintorDusko AmintorDusko AmintorDusko approved these changes

@multiphaseCFD multiphaseCFD multiphaseCFD approved these changes

@josephleekl josephleekl Awaiting requested review from josephleekl

@maliasadi maliasadi Awaiting requested review from maliasadi

Assignees
No one assigned
Labels
ci:use-gpu-runner Enable usage of GPU runner for this Pull Request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@LuisAlfredoNu @multiphaseCFD @maliasadi @AmintorDusko