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

fixed indentation in VQE expectation #157

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rsln-s
Copy link

@rsln-s rsln-s commented May 21, 2018

Fixed incorrect indentation introduced in eb476e2

@rsln-s rsln-s changed the title fixed indentation fixed indentation in VQE expectation May 21, 2018
@vtomole
Copy link
Contributor

vtomole commented May 22, 2018

@rsln-s Why does the indentation need fixing? This commit is not building due to

CMake Error at CMakeLists.txt:2 (cmake_minimum_required):
      CMake 3.2 or higher is required.  You are running version 2.8.12.2
    
    
    -- Configuring incomplete, errors occurred!
    Error: could find generator in Cache
    error: [Errno 2] No such file or directory: 'osqp_sources/build/out/libosqpstatic.a'

@rsln-s
Copy link
Author

rsln-s commented May 22, 2018

@vtomole Two reasons:

  1. Performance. With current indentation the line
meas_outcome = expectation_from_sampling(pyquil_prog + meas_basis_change,
                                                      qubits_to_measure,
                                                      qvm,
                                                      samples)

is called more times than needed, with some results being thrown out, which means more calls to qvm.
For example, for term (0.5+0j)*Z0*Z1, expectation_from_sampling(..) is called twice (first for qubits_to_measure = [0], second for qubits_to_measure = [0,1]), and first meas_outcome is thrown out. Calls to QVM being one of the most costly parts of QAOA and VQE, this is wasteful.

  1. Understandability. Incorrect indentation obfuscates the logic of computing expectation by term and makes the code hard to understand and extend.

Regarding building problem: not sure what's wrong with that. Running tests with tox doesn't work on my machine either, even for master:

...
Command "/home/ruslan/dev/grove-fork/.tox/py36/bin/python3.6 -u -c "import setuptools, tokenize;__file__='/tmp/pip-install-r872urds/osqp/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" install --record /tmp/pip-record-q0i38ry7/install-record.txt --single-version-externally-managed --compile --install-headers /home/ruslan/dev/grove-fork/.tox/py36/include/site/python3.6/osqp" failed with error code 1 in /tmp/pip-install-r872urds/osqp/
ERROR: InvocationError for command '/home/ruslan/dev/grove-fork/.tox/py36/bin/pip install cvxpy' (exited with code 1)
___________________________________________________________________________________________________________________________________ summary ____________________________________________________________________________________________________________________________________
ERROR:   py27: InvocationError for command /home/ruslan/dev/grove-fork/.tox/py27/bin/pip install /home/ruslan/dev/grove-fork/.tox/dist/quantum-grove-1.6.0.zip (see /home/ruslan/dev/grove-fork/.tox/py27/log/py27-2.log) (exited with code 1)
ERROR:   py36: commands failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants