-
Notifications
You must be signed in to change notification settings - Fork 56
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
Numba permanent #300
Numba permanent #300
Conversation
Added Ryser algorithm to find the permanent of a matrix. Note that perm_ryser(A) can find the permanent of a real matrix as well as the permanent of a complex one.
Jqz1111 patch 1
Changed to possibly return ryser and bbfg instead of complex and real ryser and bbfg
Update _permanent.py
Used black and pylint
Update libwalrus.pyx
Removed imports of "perm_real", "perm_complex", "perm_BBFG_real" and "perm_BBFG_complex".
…s into Jiaqi_cpp_removal
Corrected function names to be only perm.
Removed #include <permanent.hpp>
Jiaqi cpp removal
|
||
* [Jiaqi Zhao](https://github.com/JQZ1111) (Polytechnique Montréal) - Photon squeezer | ||
|
||
* [Dominic Leclerc](https://github.com/dleclerc33) (Polytechnique de Montréal) :fleur_de_lis: King of vaccum | ||
|
||
* [Benjamin Lanthier](https://github.com/benjaminlanthier) (Polytechnique de Montréal) :mage: Warlock of eigenvalues | ||
|
||
* [Brandon Turcotte](https://github.com/brandonpolymtl) (Polytechnique de Montréal) - :star2: The Quantum-plators |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎆 🏅 🎊
thewalrus/_permanent.py
Outdated
Permanent Python interface | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This need to have a little bit more documentation as for example in lines 14-36 of https://github.com/XanaduAI/thewalrus/blob/master/thewalrus/fock_gradients.py
Co-authored-by: Josh Izaac <josh146@gmail.com>
thewalrus/_permanent.py
Outdated
if a != b: | ||
raise Exception("Not a square matrix") | ||
n = len(M) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@josh146 : how do feel about raising exception from numba code? Note that these checks are also done by wrapper function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no great opinion 🙂 If the checks are already being done by the wrapper function, makes sense to remove it from here though I would think? Saves the validation being done twice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note though it's best not to raise a bare Exception
, but to always use a subclass e.g., ValueError
[sc-9848] |
Tried to respect correct indentation
Modifications header
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me @nquesada! Only one question about the tests and the bbfg method :)
perm_BBFG_real = lambda x: perm(x, method="bbfg") | ||
perm_BBFG_complex = lambda x: perm(x, method="bbfg") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these used anywhere below? Since it looks like the rest of the test file is not modified 🤔
Oh also don't forget to add an item to the changelog before merging! |
Implements the Ryser and BBFG algorithms for the calculation of the permanent using numba.