Fix sampling when using circuit.add()

[andrea-pasquale]

Closes #563.
As already discussed in today's meeting, the problem was related to the add method of AbstractCircuit.
When the new circuit is created the repeated_execution attribute, which allows to re-execute the circuit for every
shot, was set by default to False. This is why when adding the noisy circuit with its inverse the sampling was odd.
Now repeated_execution is set to:

newcircuit.repeated_execution = old_circuit.repeated_execution or circuit_to_add.repeated_execution

Therefore if one of the two circuits requires to re-execute the circuit the new circuit will also be re-executed for every shot.
I've also added tests about this.
Let me know what you think @igres26 @scarrazza @stavros11.

[stavros11]

Thanks for finding the issue. I believe this solves the problem, just two comments below.

[codecov]

Codecov Report

Merging #564 (ed0f9e7) into master (e66224d) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #564   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           84        84           
  Lines        12681     12704   +23     
=========================================
+ Hits         12681     12704   +23     

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/abstractions/circuit.py	100.00% <100.00%> (ø)
src/qibo/tests/test_core_circuit_noise.py	100.00% <100.00%> (ø)

---

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 e66224d...ed0f9e7. Read the comment docs.

[andrea-pasquale]

I've implemented a test that checks the sampling after fixing the seed (using K.set_seed(123).
However, I've noticed that the parameter seed of the PauliNoiseChannel does not affect the results of the sampling. That's why here I've decided to omit it. Is this the correct behaviour, @stavros11 ?

[stavros11]

Thanks for adding the test.

However, I've notice that the parameter seed of the PauliNoiseChannel does not affect the results of the sampling. That's why here I've decided to omit it. Is this the correct behaviour, @stavros11 ?

Can you give an example where it doesn't affect? For example, the following script:

from qibo import models, gates
import numpy as np


circ = models.Circuit(1)
circ_no_noise = models.Circuit(1)

for _ in range(10):
    new_gate = gates.H(0)
    circ.add(gates.PauliNoiseChannel(0, pz=0.1))
    circ.add(new_gate)
    circ_no_noise.add(new_gate)

circ.add(gates.PauliNoiseChannel(0, pz=0.1))
circ += circ_no_noise.invert()
circ.add(gates.M(0))

print(circ(nshots=100).samples()[:, 0])

gives me different samples every time I run, while if I add a seed in the last noise channel, it gives me the same samples every time.

[stavros11]

It doesn't really matter for testing purposes, but the last noise channel is outside the for loop in #563. Also it is fine to use a smaller circuit for testing (eg. less than 100 gates).

[andrea-pasquale]

Yes, I agree. I can lower it down to 10.
EDIT: I will also move the last noise out of the loop, thanks for spotting this.

[stavros11]

Thanks for adding the test.

However, I've notice that the parameter seed of the PauliNoiseChannel does not affect the results of the sampling. That's why here I've decided to omit it. Is this the correct behaviour, @stavros11 ?

Can you give an example where it doesn't affect? For example, the following script:

from qibo import models, gates
import numpy as np


circ = models.Circuit(1)
circ_no_noise = models.Circuit(1)

for _ in range(10):
    new_gate = gates.H(0)
    circ.add(gates.PauliNoiseChannel(0, pz=0.1))
    circ.add(new_gate)
    circ_no_noise.add(new_gate)

circ.add(gates.PauliNoiseChannel(0, pz=0.1))
circ += circ_no_noise.invert()
circ.add(gates.M(0))

print(circ(nshots=100).samples()[:, 0])

gives me different samples every time I run, while if I add a seed in the last noise channel, it gives me the same samples every time.

[andrea-pasquale]

Can you give an example where it doesn't affect?

I've noticed it with the test that I've added, but I can reproduce it also using your example.
If I add K.set_seed than the seed in the noise is ignored. For example I can choose a random seed for each gate as shown here

from qibo import models, gates, K
import numpy as np

K.set_seed(123)

circ = models.Circuit(1)
circ_no_noise = models.Circuit(1)

for i in range(10):
    new_gate = gates.H(0)
    circ.add(gates.PauliNoiseChannel(0, pz=0.1, seed=i))
    circ.add(new_gate)
    circ_no_noise.add(new_gate)

circ.add(gates.PauliNoiseChannel(0, pz=0.1))
circ += circ_no_noise.invert()
circ.add(gates.M(0))

print(circ(nshots=100).samples()[:, 0])

But if I set the seed at the beginning the output remains the same.

[stavros11]

The example from the last post gives me

[Qibo 0.1.8.dev0|INFO|2022-03-25 15:50:00]: Using qibojit (numba) backend on /CPU:0
[0 1 0 0 0 1 0 0 1 0 0 0 0 1 1 1 1 0 0 0 1 1 1 0 0 0 0 0 1 0 0 0 0 1 0 0 1
 1 0 0 0 0 0 0 0 1 1 0 0 1 1 0 0 0 1 0 0 0 0 1 0 0 0 1 0 0 0 0 0 1 1 1 1 0
 0 0 1 1 0 0 1 1 1 0 1 0 0 0 0 0 0 1 1 0 0 1 0 0 0 0]

every time I run the script, regardless of whether I use K.set_seed(123) in the beginning. Do you get something different?

If I read the code correctly, using seed=... when defining the channel just sets the numpy random seed at that time. So any seed setting you do before that will not have any effect, only the last seed setting will matter.

Regarding the tests, I still get a failure for qibojit-cupy and tensorflow, but now because the output differs from the target. Have you tested in an environment with tensorflow or cupy?
This is because K.set_seed sets the seed only for the active backend (tensorflow or cupy resepectively), while PauliNoiseChannel uses np.random when deciding which Pauli to apply, regardless of the active backend. Adding a np.random.seed before each circuit execution in the test solves the issue for me.

[andrea-pasquale]

Regarding the tests, I still get a failure for qibojit-cupy and tensorflow, but now because the output differs from the target. Have you tested in an environment with tensorflow or cupy?
This is because K.set_seed sets the seed only for the active backend (tensorflow or cupy resepectively), while PauliNoiseChannel uses np.random when deciding which Pauli to apply, regardless of the active backend. Adding a np.random.seed before each circuit execution in the test solves the issue for me.

I understand. I've implemented the fix and now tests are passing for all backends locally on cpu and gpu. Thanks!

The example from the last post gives me
[Qibo 0.1.8.dev0|INFO|2022-03-25 15:50:00]: Using qibojit (numba) backend on /CPU:0
[0 1 0 0 0 1 0 0 1 0 0 0 0 1 1 1 1 0 0 0 1 1 1 0 0 0 0 0 1 0 0 0 0 1 0 0 1
1 0 0 0 0 0 0 0 1 1 0 0 1 1 0 0 0 1 0 0 0 0 1 0 0 0 1 0 0 0 0 0 1 1 1 1 0
0 0 1 1 0 0 1 1 1 0 1 0 0 0 0 0 0 1 1 0 0 1 0 0 0 0]
every time I run the script, regardless of whether I use K.set_seed(123) in the beginning. Do you get something different?

Yes I get the same here. Instead if I do something like this:

from qibo import models, gates, K
import numpy as np

K.set_seed(123)

circ = models.Circuit(1)
circ_no_noise = models.Circuit(1)

for i in range(10):
    new_gate = gates.H(0)
    circ.add(gates.PauliNoiseChannel(0, pz=0.1, seed=np.random.randint(100)))
    circ.add(new_gate)
    circ_no_noise.add(new_gate)

circ.add(gates.PauliNoiseChannel(0, pz=0.1))
circ += circ_no_noise.invert()
circ.add(gates.M(0))

print(circ(nshots=5).samples()[:, 0])

The result is always the same only if I put the set_seed at the beginning, which makes sense since I am fixing the numpy seed at the beginning with K.set_seed.

[stavros11]

Thanks, everything works for me too now.

The result is always the same only if I put the set_seed at the beginning, which makes sense since I am fixing the numpy seed at the beginning with K.set_seed.

Yes, but I believe this is expected because you are using a random seed in the PauliNoiseChannel and K.set_seed is fixing this. If you don't use a random seed but a fixed number, the channel seed should work and give the same results in every run.