More use of sympy internally #196
Conversation
|
I was using this version in a project and one of the executions in ibmqx4 I received the following message qiskit._resulterror.ResultError: QASM_NOT_VALID: Error parsing QASM. Error parsing qasm number 0. Parse error on line 5: Edit: @lbello thank you very much for your work to improve qiskit. Here is the file that generated the above bug. Interestingly, if line 30 is moved to line 39 the bug does not occur. Should I include a test? |
| @@ -692,8 +699,6 @@ def optimize_1q_gates(circuit): | |||
| right_parameters[0], | |||
| right_parameters[1], | |||
| right_parameters[2]) | |||
| # Evaluate the symbolic expressions for efficiency | |||
| right_parameters = tuple(map(sympy.N, list(right_parameters))) | |||
There was a problem hiding this comment.
If we simplify a long sequence of u3 gates, is there concern that the expression size will grow? We can test this.
qiskit/unroll/_jsonbackend.py
Outdated
| 'params': [arg[0].real(nested_scope), | ||
| arg[1].real(nested_scope), | ||
| arg[2].real(nested_scope)], | ||
| 'params': [arg[0].sym(nested_scope), |
There was a problem hiding this comment.
The comment at the top of the file needs to be updated to be consistent.
qiskit/unroll/_jsonbackend.py
Outdated
| 'params': [arg[0].real(nested_scope), | ||
| arg[1].real(nested_scope), | ||
| arg[2].real(nested_scope)], | ||
| 'params': [arg[0].sym(nested_scope), |
There was a problem hiding this comment.
Actually I have a doubt about whether these should remain floats or become syms until we verify backend behavior. Perhaps related to the example @adjs gave in the conversation for this PR?
add flag to dagcircuit.qasm to optionally evaluate symbols. make jsonbackend use real() for now. make compile() write qasm with evaluated symbols for QX. revert changes to optimization.py. change latex() methods of expession nodes to use sympy.
- revert an is_zero comparison. There are circuits that cause us to reach the assertion in yzy_to_zyz when is_zero is used but not when an approximate comparison is used. The circuits have u3 gates with random parameters. - cast from Float to float in jsonbackend.
use a fixed seed when compile() calls swap_mapper(). update the test for Python 3.5 and 3.6.
There are reasonable examples where simplification takes too long. The time is dramatically reduced by evaluating the symbolic expressions before computing the produce of u3 gates. Also added a TODO comment to QuantumProgram.
one of the new tests was failing after the last commit because the evaluation introduced approximate results. reverting is_zero to approximately zero is a solution. speed was the same with only one evalf() before compose_u3.
|
This last set of commits 1) corrected a few issues that arise for some common examples, mainly issues with using is_zero 2) added a new mapper test that highlights the problem and 3) opted to call evalf() prior to composing u3 operators due to the massive difference in time between symbolic and numeric solution. We can aim to cleanly separate approximate and exact rewriting rules as we go forward. I think the PR is ready for review and possible merging. |
|
Thanks everyone involved in this large PR - it's looking ready for merging indeed! |
* requirements: sympy>=1.0 * use sym for jsonbackend * merge with Qiskit#171 * sympy in the simulator * optimizing the simulator * test readjustment * json serialization of symbolic types * RYGate with symbolic param * symbolic conversion higher in the heritage chain * test_apps testing something, not just running * a cleaner logger * sympy Hamiltonian and TestHamiltonian * dag using sym * sympy in extensions and more compact tests * test readjustment * N is for getting numbers * reformat code * cleaning before PR * cleaning up tests * merge * constant pi * numbers are Numbers * pi constant * tests with pi and bugfixing * t uses pi * s uses pi * reals are sympy.Numbers, not floats * mapper without epsilon and circuitbackend with sym * adjusting tests * circuitbackend: real -> sym * printerbackend.py: real->sym * spelling * style! * several changes to address issues. add flag to dagcircuit.qasm to optionally evaluate symbols. make jsonbackend use real() for now. make compile() write qasm with evaluated symbols for QX. revert changes to optimization.py. change latex() methods of expession nodes to use sympy. * update tests. * fix problems. - revert an is_zero comparison. There are circuits that cause us to reach the assertion in yzy_to_zyz when is_zero is used but not when an approximate comparison is used. The circuits have u3 gates with random parameters. - cast from Float to float in jsonbackend. * add circuit test with random parameters. * add test file. * test for python3.5. * add seed to swap_mapper. use a fixed seed when compile() calls swap_mapper(). update the test for Python 3.5 and 3.6. * evaluate before and after compose_u3. There are reasonable examples where simplification takes too long. The time is dramatically reduced by evaluating the symbolic expressions before computing the produce of u3 gates. Also added a TODO comment to QuantumProgram. * evaluate before compose_u3 and revert more is_zero. one of the new tests was failing after the last commit because the evaluation introduced approximate results. reverting is_zero to approximately zero is a solution. speed was the same with only one evalf() before compose_u3. * bug! ups...
This is part of the effort to extend the use of sympy internally. See #158 for details