Fix scalar mutliplication of SymbolicTerm
#474
Conversation
Codecov Report
@@ Coverage Diff @@
## master #474 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 84 85 +1
Lines 11770 11905 +135
==========================================
+ Hits 11770 11905 +135
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
I simplified a little bit the |
src/qibo/core/terms.py
Outdated
| matrix_map (dict): Dictionary that maps target qubit ids to a list of | ||
| matrices that act on each qubit. | ||
| """ | ||
| """Helper constructor using the ``sympy`` expression directly. |
There was a problem hiding this comment.
I think there is something wrong with this comment, no?
There was a problem hiding this comment.
Yes, thanks. I was going to update the docstring after removing the .from_factors constructor for simplification but I forgot to complete the update. It should be ok now.
|
|
||
| if isinstance(factor, sympy.Symbol): | ||
| if isinstance(factor.matrix, K.qnp.tensor_types): | ||
| self.factors.extend(pow * [factor]) |
There was a problem hiding this comment.
Given that the comments are quite good, please some sentence explaining how and why the pow term is applied to the matrix_map.
Thank you for the review. I update the comments for the above points, let me know if it is ok. |
Fixes #470, the fix was very simple and is in the first commit only. In the rest I am adding some unit tests that focus on the
core/terms.pyfile and improving the related docstrings and comments to help in the future, because as it is now it was quite hard to debug this issue.