search

XanaduAI / thewalrus

A library for the calculation of hafnians, Hermite polynomials and Gaussian boson sampling.
https://the-walrus.readthedocs.io
Apache License 2.0
100 stars 55 forks source link

takagi fix #394

Closed RyosukeNORO closed 1 month ago

RyosukeNORO commented 1 month ago

Context: There were issues with Takagi decomposition, specifically sqrtm, with some matrix. For the previous discussion, please see (https://github.com/XanaduAI/thewalrus/pull/393)

Description of the Change: added the calculation method when the matrix to be takagi decomposed is diagonal. added the test code of the matrix for which the decomposition did not work well to test_decompositions.py.

Benefits: Takagi decomposition works well.

Possible Drawbacks:

Related GitHub Issues: (https://github.com/XanaduAI/thewalrus/pull/393)

elib20 commented 1 month ago

@timmysilv, what do you suggest we do about codefactor here? We have a function with a lot of edge cases where the computation simplifies, so we'd like to leverage that, but now it is complaining about too many return statements.

nquesada commented 1 month ago

@elib20 : for the complaint about too many returns you add #pylint ignore to tell it to shut up ;)

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (f935053) to head (b5fbd06).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #394 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 28 28 Lines 1996 1912 -84 ========================================= - Hits 1996 1912 -84 ``` | [Files](https://app.codecov.io/gh/XanaduAI/thewalrus/pull/394?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI) | Coverage Δ | | |---|---|---| | [thewalrus/decompositions.py](https://app.codecov.io/gh/XanaduAI/thewalrus/pull/394?src=pr&el=tree&filepath=thewalrus%2Fdecompositions.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI#diff-dGhld2FscnVzL2RlY29tcG9zaXRpb25zLnB5) | `100.00% <100.00%> (ø)` | | ... and [25 files with indirect coverage changes](https://app.codecov.io/gh/XanaduAI/thewalrus/pull/394/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI) ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/XanaduAI/thewalrus/pull/394?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/XanaduAI/thewalrus/pull/394?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI). Last update [f935053...b5fbd06](https://app.codecov.io/gh/XanaduAI/thewalrus/pull/394?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI).
nquesada commented 1 month ago

Almost there @RyosukeNORO In the test that you added you need to add an extra line to trigger the lines where the order of the takagi values and unitaries are flipped. With that you should get full coverage.