`QulacsBackend` #1272

BrunoLiegiBastonLiegi · 2024-03-18T14:10:18Z

This adds qulacs as an optional backend.

Checklist:

Reviewers confirm new code works as expected.
Tests are passing.
Coverage does not decrease.
Documentation is updated.

codecov · 2024-03-19T09:04:20Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.84%. Comparing base (dd3c885) to head (6033640).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1272   +/-   ##
=======================================
  Coverage   99.84%   99.84%           
=======================================
  Files          75       76    +1     
  Lines       10807    10844   +37     
=======================================
+ Hits        10790    10827   +37     
  Misses         17       17

Flag	Coverage Δ
unittests	`99.84% <100.00%> (+<0.01%)`	⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

BrunoLiegiBastonLiegi · 2024-03-21T10:38:14Z

@scarrazza some preliminary results for the qulacs backend, it looks indeed comparable with our `qibojit

1 core

8 cores

scarrazza · 2024-03-25T00:43:17Z

Thanks @BrunoLiegiBastonLiegi, could you please include some GPU curves too?

BrunoLiegiBastonLiegi · 2024-03-25T07:10:58Z

Yes I am trying to install the qulacs-gpu package on the cluster, but it requires a little bit of machinery. The pip install qulacs-gpu doesn't work, thus I am not sure how we could set that as an optional dependency.

scarrazza

Looks good, however we are missing documentation.

BrunoLiegiBastonLiegi · 2024-04-30T10:44:31Z

I added a small paragraph in the documentation about qulacs, there is not much to say as at this level we are supporting the basic features of qulacs only. What still needs to updated is the scheme in the getting started section that summarizes all the available backends. @MatteoRobbiati can I ask you hand with that?

src/qibo/backends/qulacs.py

alecandido

Thanks @BrunoLiegiBastonLiegi for the effort!

src/qibo/backends/__init__.py

alecandido · 2024-06-24T13:48:50Z

tests/test_measurements.py

@@ -435,7 +435,7 @@ def test_measurement_basis_list(backend):
    c.add(gates.H(2))
    c.add(gates.X(3))
    c.add(gates.M(0, 1, 2, 3, basis=[gates.X, gates.Z, gates.X, gates.Z]))
-    result = c(nshots=100)
+    result = backend.execute_circuit(c, nshots=100)


I like a lot the explicit backend usage, but why do you need it for this PR?

Because otherwise only the GlobalBackend is tested here, if I am not mistaken. Anyway, this was failing with the QulacsBackend, but was not supposed to be testing it, even though it did due to the GlobalBackend usage.

I thought that @stavros11 set these tests to run with all the available backends, through a mechanism implemented in conftest.py

qibo/tests/conftest.py

Lines 63 to 87 in c68e58b

@pytest.fixture

def backend(backend_name):

yield get_backend(backend_name)

def pytest_generate_tests(metafunc):

module_name = metafunc.module.__name__

if module_name == "tests.test_models_distcircuit_execution":

config = [(bk, acc) for acc in ACCELERATORS for bk in MULTIGPU_BACKENDS]

metafunc.parametrize("backend_name,accelerators", config)

else:

if "backend_name" in metafunc.fixturenames:

if "accelerators" in metafunc.fixturenames:

config = [(backend, None) for backend in AVAILABLE_BACKENDS]

config.extend(

(bk, acc) for acc in ACCELERATORS for bk in MULTIGPU_BACKENDS

)

metafunc.parametrize("backend_name,accelerators", config)

else:

metafunc.parametrize("backend_name", AVAILABLE_BACKENDS)

elif "accelerators" in metafunc.fixturenames:

metafunc.parametrize("accelerators", ACCELERATORS)

It's true that is using the GlobalBackend, but it should be also setting the backend registered there to all the possible backends.
So, the backend.execute_circuit(c, *args) should be fully equivalent to have the backend fixture calling set_backend(), and use c(*args).

However, that's not the case, but I'm worried that all tests should be updated to be consistent, unless they are calling set_backend() within the test.

One way or the other, I'd like if there were a unique way of doing things. And in this PR, I'd use the current one, even if it's suboptimal.
If there is truly a better way (which most likely it's true, and it will be the one you're using, or, slightly worse, changing the backend fixture to set the backend as well), we should open a PR right after this one to implement it consistently over all the tests that are backend-dependent.

What do you think? @BrunoLiegiBastonLiegi @stavros11

I thought that @stavros11 set these tests to run with all the available backends, through a mechanism implemented in conftest.py

If I remember correctly, the backend fixture does not set the GlobalBackend. Therefore

c(nshots=100)

in a test that uses the backend fixture is not equivalent to

backend.execute_circuit(c, nshots=100)

The second will indeed test all different available backends, while the first will only test whatever GlobalBackend was set at that time. Actually, it will probably repeat the test using the same GlobalBackend multiple times, because the fixture will still loop over all available backends.

The GlobalBackend in this case is not very well defined, because if a previous test switches it (using set_backend) then this value will be used in all upcoming tests, until it is switched by another test. This is usually bad (as is the idea of GlobalBackend, and more generally global variables), because tests that are supposed to be independent actually depend on what happened in previous tests.

I think the general idea is to avoid using the GlobalBackend in tests, except for very few tests that exist for testing the GlobalBackend functionality itself. All tests of other features should use the backend.execute_circuit approach. If a test needs to use the GlobalBackend, this should ideally be done as:

def test_...(): original_backend = qibo.get_backend() qibo.set_backend("backend to test") # test code here qibo.set_backend(original_backend)

We could also wrap this to another (or the same?) fixture, but I don't think that exists right now.

No it doesn't, indeed I wrote it (in a bit convoluted way) in my previous comment (that was mainly hypothetical, eventually negating the hypothesis...).

I actually searched, and only a few tests are actually using set_backend(), most of them just to test it.

Tests using set_backend

❯ : rg set_backend test_backends_qibotn.py 13: qibo.set_backend(backend="qibotn", platform="qutensornet", runcard=None) 16: qibo.set_backend("numpy") test_backends_clifford.py 8:from qibo import Circuit, gates, set_backend 35:def test_set_backend(backend): 38: set_backend("clifford", platform=platform) 46: set_backend(backend.name, platform=backend.platform) 52: set_backend("numpy") test_models_qcnn.py 319: qibo.set_backend("qibojit") test_backends_global.py 7:def test_set_backend(): 11: qibo.set_backend("numpy") 31: qibo.set_backend("numpy") 46: qibo.set_backend("numpy") 83: qibo.set_backend("numpy") 91: qibo.set_backend("numpy") test_models_dbi.py 6:from qibo import hamiltonians, set_backend test_backends.py 6:from qibo import construct_backend, gates, list_available_backends, set_backend 130:def test_set_backend_error(): 132: set_backend("non-existing-backend")

Many more are already using backend.execute_circuit(), against my expectation.

So, fine, this PR is aligning to the majority, and that's good. Sorry for doubting it.
We should just fix the other tests (like test_models_qcnn.py) or remove the import where is not even used (i.e. test_models_dbi.py, ...).

I actually searched, and only a few tests are actually using set_backend(), most of them just to test it.

This is fine, but there are also tests that are using circuit() without set_backend(). These are also invoking the GlobalBackend to execute the circuit and are even worse, because the GlobalBackend will be whetever was set beforehand (by the last set_backend() of a previous test). These may also be a bit harder to search for, because they can appear in various ways: circuit(), circuit.execute(), c(), whatever_circuit_name(), c(nshots=100) (like the one fixed here), etc.. An easy way would be to remove Circuit.__call__ and Circuit.execute methods and see which tests fail.

By the way, I think that this discussion is not very relevant to this PR and maybe should be converted to an issue.

By the way, I think that this discussion is not very relevant to this PR and maybe should be converted to an issue.

I agree about the issue. If you wish, feel free to open yourself, you're definitely the most expert.

An easy way would be to remove Circuit.__call__ and Circuit.execute methods and see which tests fail.

Another option is to keep Circuit.__call__, but require a backend argument to be passed.

I agree about the issue. If you wish, feel free to open yourself, you're definitely the most expert.

#1374

Do we have a list of tests relying on a random state of the GlobalBackend?

Do we have a list of tests relying on a random state of the GlobalBackend?

These are the tests that execute circuits using the circuit.execute interface, which invokes the GlobalBackend():

test_backends_global::test_circuit_execution (sets to numpy)

test_measurements::test_measurement_compiled_circuit (does not set)

test_models_circuit_execution::test_compiled_execute (does not set)

Various tests in test_models_qcnn are using the GlobalBackend but setting it beforehand to qibojit or numpy without reseting it after.

test_models_variational::test_vqe (does not set)

test_models_variational::test_custom_loss (does not set)

src/qibo/backends/qulacs.py

Co-authored-by: Stavros Efthymiou <[email protected]>

…he backend

stavros11

Thanks @BrunoLiegiBastonLiegi, looks good to me and seems to work after the latest fixes.

stavros11 · 2024-06-27T17:09:34Z

src/qibo/backends/qulacs.py

+import qulacs  # pylint: disable=import-error
+from qulacs import (  # pylint: disable=no-name-in-module, import-error


Do we need this exceptions because otherwise pylint fails for mac with py3.9 in the CI? If yes, I guess that's fine, otherwise I am not sure if they are needed given that CI is using the test dependencies which include qulacs.

Yes, if I remember correctly pylint was complaining in macos-python3.9

I'd say this is definitely the best way you could do it. At least is confined within the qibo.backends.qulacs module, though not restricted to the offending macos+py3.9 combo.

If done in the workflow, it would be positioned in a larger scope, though you may be able to ignore specifically that file for that combo.

stavros11 · 2024-06-27T17:12:07Z

tests/test_models_qcnn.py

@@ -13,6 +13,7 @@

 def test_classifier_circuit2():
    """ """
+    set_backend("numpy")


Following the discussion above, this is also not very good practice because it will also set numpy as the GlobalBackend for all upcoming tests. However, since everything should work with numpy, it will probably not cause any issue. Maybe we could just open an issue for the test discussion and postpone for now.

Yeah, I agree, however the QCNN model doesn't seem to support the specification of the execution backend and always uses the GlobalBackend, I opened an issue about this already #1362. The tests were originally using the NumpyBackend (the global one) and thus I set it explicitely here.

alecandido

Just one last tiny comment. Other than that, it is good for me.

Thanks @BrunoLiegiBastonLiegi!

src/qibo/backends/qulacs.py

alecandido · 2024-06-29T08:45:42Z

@BrunoLiegiBastonLiegi whenever the CI passes, feel free to merge :)

(just make sure before that your branch is up-to-date with master, other than lacking conflicts - better to test merges in the PRs than in master)

alecandido · 2024-06-29T08:46:15Z

@scarrazza could you withdraw your review?

BrunoLiegiBastonLiegi added 10 commits March 18, 2024 18:08

feat: added qualcs backend

02325c6

feat: removed cache files

20e03a9

feat: added test

41f1117

fix: removed cache file

d6a51cc

build: added qulacs as an optional dependency

e8e3558

fix: removed the optimize option

a16e40e

build added test dependency

32a4ae9

feat: added qulacs to set_backend

eab30fc

fix: disabled some pylint errors

2bc412b

fix: fixed order of qubits in the state

c5e1b39

BrunoLiegiBastonLiegi added 2 commits March 19, 2024 14:33

fix: improved coverage

4f0aacb

fix: removed the possibility to set an initial state

bb7417b

BrunoLiegiBastonLiegi marked this pull request as ready for review March 20, 2024 05:33

BrunoLiegiBastonLiegi requested a review from scarrazza March 20, 2024 05:33

BrunoLiegiBastonLiegi mentioned this pull request Mar 25, 2024

Publish GPU package qulacs/qulacs#624

Open

MatteoRobbiati assigned BrunoLiegiBastonLiegi Mar 27, 2024

scarrazza requested changes Apr 24, 2024

View reviewed changes

doc: added a paragraph about qulacs in the docs

9b9e1a7

BrunoLiegiBastonLiegi added 5 commits April 30, 2024 12:53

feat: added some dosctrings

8ffa7f7

build: updated lock

1a642ef

Merge branch 'master' into qulacs

f8abd22

build: merge master

d2a9a46

build: merge master + lock update

02fb9f2

MatteoRobbiati self-assigned this May 8, 2024

doc: added qulacs bullet in getting_started section

5ecc5d8

BrunoLiegiBastonLiegi added 5 commits June 11, 2024 14:26

fix: I love pylint

885c680

fix: added qulacs to list available backends test

7f51cda

build: merge master

89b0997

fix: fixing various global backend related tests

3c8cccf

fix: fix list available backends test

a579c1f

MatteoRobbiati requested a review from alecandido June 19, 2024 09:13

alecandido requested a review from stavros11 June 19, 2024 09:15

MatteoRobbiati added this to the Qibo 0.2.9 milestone Jun 19, 2024

stavros11 reviewed Jun 24, 2024

View reviewed changes

src/qibo/backends/qulacs.py Outdated Show resolved Hide resolved

alecandido reviewed Jun 24, 2024

View reviewed changes

scarrazza modified the milestones: Qibo 0.2.9, Qibo 0.2.10 Jun 25, 2024

BrunoLiegiBastonLiegi and others added 4 commits June 25, 2024 15:39

Update src/qibo/backends/qulacs.py

b7c32ca

Co-authored-by: Stavros Efthymiou <[email protected]>

feat: raise error with initial_state + removed some attributes from t…

9a7fdaa

…he backend

fix: restored the simulator and converter attributes

94d5c89

fix: removed attributes and moved qulacs imports

f29ae65

stavros11 approved these changes Jun 27, 2024

View reviewed changes

stavros11 reviewed Jun 27, 2024

View reviewed changes

stavros11 mentioned this pull request Jun 27, 2024

Testing different backends #1374

Open

alecandido approved these changes Jun 28, 2024

View reviewed changes

src/qibo/backends/qulacs.py Outdated Show resolved Hide resolved

BrunoLiegiBastonLiegi added 2 commits June 29, 2024 09:13

remove qulacs from attributes

27dac43

removing self.qulacs calls

a59041d

Merge branch 'master' into qulacs

6033640

scarrazza approved these changes Jun 29, 2024

View reviewed changes

BrunoLiegiBastonLiegi added this pull request to the merge queue Jun 29, 2024

Merged via the queue into master with commit 6316889 Jun 29, 2024
27 checks passed

BrunoLiegiBastonLiegi deleted the qulacs branch June 29, 2024 09:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`QulacsBackend` #1272

`QulacsBackend` #1272

BrunoLiegiBastonLiegi commented Mar 18, 2024

codecov bot commented Mar 19, 2024 •

edited

Loading

BrunoLiegiBastonLiegi commented Mar 21, 2024 •

edited

Loading

scarrazza commented Mar 25, 2024

BrunoLiegiBastonLiegi commented Mar 25, 2024

scarrazza left a comment

BrunoLiegiBastonLiegi commented Apr 30, 2024

alecandido left a comment

alecandido Jun 24, 2024

BrunoLiegiBastonLiegi Jun 25, 2024

alecandido Jun 26, 2024 •

edited

Loading

stavros11 Jun 26, 2024 •

edited

Loading

alecandido Jun 26, 2024 •

edited

Loading

stavros11 Jun 27, 2024 •

edited

Loading

alecandido Jun 27, 2024

stavros11 Jun 27, 2024

scarrazza Jun 28, 2024

stavros11 Jun 28, 2024

stavros11 left a comment

stavros11 Jun 27, 2024

BrunoLiegiBastonLiegi Jun 27, 2024

alecandido Jun 28, 2024

stavros11 Jun 27, 2024

BrunoLiegiBastonLiegi Jun 27, 2024

alecandido left a comment

alecandido commented Jun 29, 2024

alecandido commented Jun 29, 2024

	@pytest.fixture
	def backend(backend_name):
	yield get_backend(backend_name)


	def pytest_generate_tests(metafunc):
	module_name = metafunc.module.__name__

	if module_name == "tests.test_models_distcircuit_execution":
	config = [(bk, acc) for acc in ACCELERATORS for bk in MULTIGPU_BACKENDS]
	metafunc.parametrize("backend_name,accelerators", config)

	else:
	if "backend_name" in metafunc.fixturenames:
	if "accelerators" in metafunc.fixturenames:
	config = [(backend, None) for backend in AVAILABLE_BACKENDS]
	config.extend(
	(bk, acc) for acc in ACCELERATORS for bk in MULTIGPU_BACKENDS
	)
	metafunc.parametrize("backend_name,accelerators", config)
	else:
	metafunc.parametrize("backend_name", AVAILABLE_BACKENDS)

	elif "accelerators" in metafunc.fixturenames:
	metafunc.parametrize("accelerators", ACCELERATORS)

		import qulacs # pylint: disable=import-error
		from qulacs import ( # pylint: disable=no-name-in-module, import-error

QulacsBackend #1272

QulacsBackend #1272

Conversation

BrunoLiegiBastonLiegi commented Mar 18, 2024

codecov bot commented Mar 19, 2024 • edited Loading

Codecov Report

BrunoLiegiBastonLiegi commented Mar 21, 2024 • edited Loading

1 core

8 cores

scarrazza commented Mar 25, 2024

BrunoLiegiBastonLiegi commented Mar 25, 2024

scarrazza left a comment

Choose a reason for hiding this comment

BrunoLiegiBastonLiegi commented Apr 30, 2024

alecandido left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alecandido Jun 26, 2024 • edited Loading

Choose a reason for hiding this comment

stavros11 Jun 26, 2024 • edited Loading

Choose a reason for hiding this comment

alecandido Jun 26, 2024 • edited Loading

Choose a reason for hiding this comment

stavros11 Jun 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stavros11 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alecandido left a comment

Choose a reason for hiding this comment

alecandido commented Jun 29, 2024

alecandido commented Jun 29, 2024

`QulacsBackend` #1272

`QulacsBackend` #1272

codecov bot commented Mar 19, 2024 •

edited

Loading

BrunoLiegiBastonLiegi commented Mar 21, 2024 •

edited

Loading

alecandido Jun 26, 2024 •

edited

Loading

stavros11 Jun 26, 2024 •

edited

Loading

alecandido Jun 26, 2024 •

edited

Loading

stavros11 Jun 27, 2024 •

edited

Loading