Mapper issue 159 #171
Merged
Mapper issue 159 #171
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
One of the tests fails in Python 3.5 but not in 3.6. The output is correct but differs from what is expected. Perhaps there is an issue with the seed. |
This was referenced Dec 2, 2017
|
I need to add a few new tests. |
Closed
It's looking good so far! As a matter of fact, I did not mark it as approved as I understand the intention is to add those new tests before marking the PR as completed? It would be extremely nice to have them, specially as this PR exercises some functionality that is not so well covered by our current test suite. I have also:
|
|
Unnecessary calls to sympy.N can increase computational cost. I have removed all calls from N to constant values in _mapping.py. awcross1#1 |
Address issue Qiskit#159. Modifications to use of sympy in optimize_1q_gates. Updated tests.
Fixes a problem with sympy integration for negated parameters.
Modify `test_math_domain_error` in order to expect different results depending on the version of Python. This is a work-around to allow the test to pass, but should be handled higher up.
33bab68 to
9192661
Compare
|
I have just rebased the PR against the current |
|
Self-reminder to comment on Qiskit/qiskit-tutorials#65 once this PR is merged. |
|
Thanks @diego-plan9 that is better. I don't have any more changes except to fix the Python 3.5 test. |
Revise "test_optimize_1q_gates_issue159" comparing the output of the compilation with two similar expected outputs.
50a7f9e to
c4b0427
Compare
diego-plan9
pushed a commit
that referenced
this pull request
Dec 21, 2017
* requirements: sympy>=1.0 * use sym for jsonbackend * merge with #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...
lia-approves
pushed a commit
to edasgupta/qiskit-terra
that referenced
this pull request
Jul 30, 2019
* optimize_1q_gates behavior change: remove u1(2*pi). Address issue Qiskit#159. Modifications to use of sympy in optimize_1q_gates. Updated tests. * Remove comment. * Add sym() method to Prefix(). Fixes a problem with sympy integration for negated parameters. * Remove print(). * Add sym() to binaryop. * new test to check the bug in Qiskit#172 * doc * Linting and style fixes * Fix test_mapper test that depends on Python vers. Modify `test_math_domain_error` in order to expect different results depending on the version of Python. This is a work-around to allow the test to pass, but should be handled higher up. * moving toffoli_gate test to test_quantumprogram * removing not necessary N () to improve performance * add new tests. * add corresponding test qasm files. * tests and corrections. * Restyle test_mapper tests, update example qasms * Allow test_mapper to compare QASM for Python 3.5 Revise "test_optimize_1q_gates_issue159" comparing the output of the compilation with two similar expected outputs.
lia-approves
pushed a commit
to edasgupta/qiskit-terra
that referenced
this pull request
Jul 30, 2019
* 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...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Change behavior of the single qubit gate optimization to remove u1(2*pi) gates.
Description
This change addresses issue #159 and corrects minor issues with some single qubit gate simplifications not occurring due to the transition to symbolic math.
Motivation and Context
This fixes issue #159 and a related issue discovered in the course of fixing the original issue.
How Has This Been Tested?
This has been tested against the example submitted in the issue and one other example. A new test was added and the existing tests were updated. All tests pass.
Screenshots (if appropriate):
Types of changes
Checklist: