XanaduAI / thewalrus

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

Tighten power-trace bound of odd loop Hafnian #362

Closed GregoryMorse closed 11 months ago

GregoryMorse commented 1 year ago

Context:: The loop Hafnian for odd sized matrices contains an excessive bound, making it a code readability issue and efficiency issue.

Description of the Change: In the inner-loop, powtrace[i // 2] reaches a maximum at n == i according to the loop range, hence the power-trace need not compute any additional power traces.

Benefits: Readability and efficiency.

Possible Drawbacks:

Related GitHub Issues:

codecov[bot] commented 1 year ago

Codecov Report

Merging #362 (73c3fa0) into master (01fb78c) will not change coverage. The diff coverage is n/a.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #362 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 27 27 Lines 1922 1922 ========================================= Hits 1922 1922 ``` | [Impacted Files](https://app.codecov.io/gh/XanaduAI/thewalrus/pull/362?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI) | Coverage Δ | | |---|---|---| | [thewalrus/\_hafnian.py](https://app.codecov.io/gh/XanaduAI/thewalrus/pull/362?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI#diff-dGhld2FscnVzL19oYWZuaWFuLnB5) | `100.00% <ø> (ø)` | | ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/XanaduAI/thewalrus/pull/362?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/362?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI). Last update [01fb78c...73c3fa0](https://app.codecov.io/gh/XanaduAI/thewalrus/pull/362?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 year ago

Pending this passing the unit tests, it has my approval.

nquesada commented 1 year ago

@GregoryMorse Could you update the CHANGELOG?

GregoryMorse commented 1 year ago

@GregoryMorse Could you update the CHANGELOG?

All set. Was looking into if a more unified treatment of the odd-sized loop Hafnians was possible, but only noticed this small improvement thus far.