Skip to content
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

Use evd driver for eigh in TwoQubitWeylDecomposition #5251

Merged
merged 6 commits into from
Oct 20, 2020

Commits on Oct 16, 2020

  1. Use eigh from numpy for TwoQubitWeylDecomposition

    The hermitian optimized eigen solver function in scipy has shown
    to possibly produce varied results in different environments. For
    example, the matrix returned by the eigh() call in the
    TwoQubitWeylDecomposition class's __init__ method when called for
    a CXGate:
    
    TwoQubitWeylDecomposition(Operator(CXGate()))
    
    was observed to be
    
    [[ 0.70710678  0.         -0.70710678  0.        ]
     [ 0.         -0.70710678  0.          0.70710678]
     [ 0.          0.70710678  0.          0.70710678]
     [ 0.70710678  0.          0.70710678  0.        ]]
    
    on one system/environment but
    
    [[ 0.70710678  0.         -0.70710678  0.        ]
     [ 0.          0.70710678  0.          0.70710678]
     [ 0.         -0.70710678  0.          0.70710678]
     [ 0.70710678  0.          0.70710678  0.        ]]
    
    on a different environment. This difference was resulting in
    inconsistent decompositions being generated and made reproducing results
    from the transpiler difficult. In testing the numpy equivalent
    function [2] has proven to be more reliable across different
    environments. This commit switches the eigh() usage in the
    TwoQubitWeylDecomposition class to use the numpy function instead
    this should fix the inconsistent decompositions we were seeing between
    environments.
    
    [1] https://docs.scipy.org/doc/scipy/reference/generated/scipy.linalg.eigh.html
    [2] https://numpy.org/doc/stable/reference/generated/numpy.linalg.eigh.html
    mtreinish committed Oct 16, 2020
    Configuration menu
    Copy the full SHA
    ac83321 View commit details
    Browse the repository at this point in the history
  2. Remove unecessary test

    This commit removes a test Qiskit#4835 which was added to validate that the
    internal values for the decomposer didn't sign flip in the specific case
    that the PR was fixing. However with the use of the numpy eigh()
    function these internal values no longer the same. So instead of
    updating the test this just deletes it since the original case it was
    catching is no longer applicable.
    mtreinish committed Oct 16, 2020
    Configuration menu
    Copy the full SHA
    149b96b View commit details
    Browse the repository at this point in the history

Commits on Oct 19, 2020

  1. Switch back to scipy.eigh and use evd driver

    This commit reverts back to using the scipy eigh() function but sets a
    the lapack driver to be fixed as evd. The numpy routine was using the
    evd driver while scipy would default to use ev as the driver. Switching
    the scipy driver from ev to evd seems to be the key difference between
    the routines which was causing the inconsistent results with scipy.
    Approaching it this way means we avoid need to bump the minimum numpy
    version in the requirements list since the numpy eigh() function was
    added in a fairly recent version.
    mtreinish committed Oct 19, 2020
    Configuration menu
    Copy the full SHA
    6d9265e View commit details
    Browse the repository at this point in the history
  2. Fix lint

    mtreinish committed Oct 19, 2020
    Configuration menu
    Copy the full SHA
    dd15274 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    c00abd1 View commit details
    Browse the repository at this point in the history

Commits on Oct 20, 2020

  1. Configuration menu
    Copy the full SHA
    2e78857 View commit details
    Browse the repository at this point in the history