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

Make qibojit default backend and update warnings #436

Merged
merged 50 commits into from
Jul 9, 2021
Merged

Conversation

stavros11
Copy link
Member

As discussed in #427 this makes qibojit the default backend if all possible backends are available. The backend order now is the following:

  1. qibojit
  2. If qibojit not available: qibotf
  3. If qibojit and qibotf not available: tensorflow
  4. If qibojit and tensorflow not available: numpy

If qibojit and qibotf are not available, a warning will appear prompting the user to install them for increased performance. If one of the two is available no warning will appear. Furthermore an info message will be printed in all cases the first time qibo is imported showing the activated benchmarks.

The user can control the logging level using the QIBO_LOG_LEVEL environment variable. Default value is 1 (everything appears) and additional options are 3 (hide info) and 4 (hide info and warnings). See here for more info on logging levels.

As we discussed, my only concern about making qibojit the default is the dry run time. Other than that this PR should be ok.

Note: I noticed that some tests are failing when running on GPU due to some cupy->numpy conversion issues. I will fix this in a different PR, it should not affect what is implemented here.

@stavros11 stavros11 requested a review from scarrazza July 4, 2021 10:17
Copy link
Member

@scarrazza scarrazza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @stavros11 looks perfect.
Let me suggest to include one sentence about the QIBO_LOG_LEVEL in the documentation.

@codecov
Copy link

codecov bot commented Jul 4, 2021

Codecov Report

Merging #436 (31c569f) into qibojit (d88a3b3) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##           qibojit      #436   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           83        83           
  Lines        11867     11865    -2     
=========================================
- Hits         11867     11865    -2     
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/qibo/tests/conftest.py 100.00% <ø> (ø)
src/qibo/backends/__init__.py 100.00% <100.00%> (ø)
src/qibo/backends/numpy.py 100.00% <100.00%> (ø)
src/qibo/config.py 100.00% <100.00%> (ø)
src/qibo/core/hamiltonians.py 100.00% <100.00%> (ø)
src/qibo/models/evolution.py 100.00% <100.00%> (ø)
src/qibo/parallel.py 100.00% <100.00%> (ø)
src/qibo/tests/test_backends_init.py 100.00% <100.00%> (ø)
src/qibo/tests/test_core_callbacks.py 100.00% <100.00%> (ø)
src/qibo/tests/test_core_distcircuit.py 100.00% <100.00%> (ø)
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d88a3b3...31c569f. Read the comment docs.

@stavros11
Copy link
Member Author

Thanks @stavros11 looks perfect.

Thanks for the review. I fixed the tests and now everything should work on both CPU and GPU.

Let me suggest to include one sentence about the QIBO_LOG_LEVEL in the documentation.

Sure, shall we do this in the backend sections that are introduced in #435? In this case I can merge the documentation branch here and update this and also the fact that qibojit is now the default backend. Let me know.

Other than that, I believe the qibojit integration is ready. We could try releasing qibojit on pip and adding it to the CI.

@scarrazza
Copy link
Member

@stavros11 thanks. Please go ahead with the log flag documentation.

@stavros11
Copy link
Member Author

@stavros11 thanks. Please go ahead with the log flag documentation.

I added a paragraph about the log level environment variable under the list of backends in the Packages section. I also made a few updates in the examples and Backends sections of the docs, as they still contained information about the defaulteinsum/matmuleinsum backends which no longer exist. @scarrazza please have a look in the updates and let me know if you agree.

@scarrazza
Copy link
Member

@stavros11 thanks, I have included an extra note about cupy installation.

@scarrazza
Copy link
Member

@stavros11 let me put this info here, I believe we should include a warning message about set_threads when if the backend is tensorflow or numpy, in fact, I believe this function does not work as expected for these backends.

@scarrazza scarrazza merged commit c35f327 into qibojit Jul 9, 2021
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.

2 participants