Skip to content
This repository has been archived by the owner on Aug 19, 2023. It is now read-only.

randomize benchmarking performace benchmark #306

Merged
merged 19 commits into from
Jul 21, 2019

Conversation

ShellyGarion
Copy link
Member

Summary

#272

Details and comments

@mtreinish
Copy link
Member

Before this can merge we'll need to make sure we're installing qiskit-ignis as part of the asv venv creation. I'll push up the modification to do that

# number of Cliffords in the sequence (start, stop, steps)
length_vector = np.arange(1, 200, 4)

params = (nq, nseeds, length_vector)
Copy link
Member

Choose a reason for hiding this comment

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

If we're not going to vary the number of qubits or the number of seeds I don't think we need to make them parameters. Was the intent to run with a different number of qubits and seeds at some point in the future?

Also this is in the wrong format. If you're using multiple params like this it's supposed to be a tuple of iterators (or lists, arrays, or other things that support iter) and then asv will build out the matrix of benchmarks to run using the combinations of all the parameters from the elements in each parameter iterator. For example you could do something like:

params = ([2,3,4,5], [20,30,40], [np.arange(1, 200, 4), np.arange(1,400, 8)])
param_names = ['nq', 'nseeds', 'length_vector']

If everything is going to be static you don't need to use params.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.


self.sim_backend = QasmSimulatorPy()

def time_simulator_transpile(self, _, __):
Copy link
Member

Choose a reason for hiding this comment

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

Also forgot to mention if you're setting 3 parameters here you'll need to add a 3rd parameter to the benchmark functions, because each parameter gets passed to setup() and each benchmark method. Since you only need to use them in setup() they're just blank here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@ajavadia
Copy link
Member

Hi is there any update on this?

@ShellyGarion
Copy link
Member Author

Hi is there any update on this?

I will continue to work on it after I finish my work on purity rb in ignis

@ShellyGarion ShellyGarion changed the title [WIP] randomize benchmarking performace benchmark randomize benchmarking performace benchmark Jun 27, 2019
Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

This is definitely getting closer, but I'm not able to run these benchmarks locally. When I attempt to by running asv dev --bench randomized everything fails with an error like:

               For parameters: 3, 10, [[0, 2], [1]], [1, 3], array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,
                      131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,
                      261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,
                      391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])
               Traceback (most recent call last):
                 File "/usr/lib/python3.7/site-packages/asv/benchmark.py", line 1039, in main_run_server
                   main_run(run_args)
                 File "/usr/lib/python3.7/site-packages/asv/benchmark.py", line 907, in main_run
                   skip = benchmark.do_setup()
                 File "/usr/lib/python3.7/site-packages/asv/benchmark.py", line 458, in do_setup
                   result = Benchmark.do_setup(self)
                 File "/usr/lib/python3.7/site-packages/asv/benchmark.py", line 390, in do_setup
                   setup(*self._current_params)
                 File "/home/mtreinish/git/qiskit/qiskit/test/benchmarks/randomized_benchmarking.py", line 76, in setup
                   seed=random_seed)
                 File "/home/mtreinish/git/qiskit/qiskit/test/benchmarks/randomized_benchmarking.py", line 50, in build_rb_circuit
                   rb_circs, _ = rb.randomized_benchmarking_seq(**rb_opts)
                 File "/usr/lib/python3.7/site-packages/qiskit/ignis/verification/randomized_benchmarking/circuits.py", line 183, in randomized_benchmarking_seq
                   qlist_flat, n_q_max = check_pattern(rb_pattern)
                 File "/usr/lib/python3.7/site-packages/qiskit/ignis/verification/randomized_benchmarking/circuits.py", line 75, in check_pattern
                   pattern_flat.extend(pat)
               TypeError: 'int' object is not iterable

except OSError:
skip_msg = ('Skipping tests for %s qubits because '
'tables are missing' % str(nq))
print(skip_msg)
Copy link
Member

Choose a reason for hiding this comment

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

In the case of an OSError we'll print this message to stdout and raise a NameError marking that benchmark as failed. Instead we should raise a NotImplementedError here to mark the benchmark as skipped (see: https://asv.readthedocs.io/en/latest/writing_benchmarks.html#setup-and-teardown-functions ):

Suggested change
print(skip_msg)
raise NotImplementedError(skip_msg)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@ShellyGarion
Copy link
Member Author

This is definitely getting closer, but I'm not able to run these benchmarks locally. When I attempt to by running asv dev --bench randomized everything fails with an error like:

               For parameters: 3, 10, [[0, 2], [1]], [1, 3], array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,
                      131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,
                      261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,
                      391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])
               Traceback (most recent call last):
                 File "/usr/lib/python3.7/site-packages/asv/benchmark.py", line 1039, in main_run_server
                   main_run(run_args)
                 File "/usr/lib/python3.7/site-packages/asv/benchmark.py", line 907, in main_run
                   skip = benchmark.do_setup()
                 File "/usr/lib/python3.7/site-packages/asv/benchmark.py", line 458, in do_setup
                   result = Benchmark.do_setup(self)
                 File "/usr/lib/python3.7/site-packages/asv/benchmark.py", line 390, in do_setup
                   setup(*self._current_params)
                 File "/home/mtreinish/git/qiskit/qiskit/test/benchmarks/randomized_benchmarking.py", line 76, in setup
                   seed=random_seed)
                 File "/home/mtreinish/git/qiskit/qiskit/test/benchmarks/randomized_benchmarking.py", line 50, in build_rb_circuit
                   rb_circs, _ = rb.randomized_benchmarking_seq(**rb_opts)
                 File "/usr/lib/python3.7/site-packages/qiskit/ignis/verification/randomized_benchmarking/circuits.py", line 183, in randomized_benchmarking_seq
                   qlist_flat, n_q_max = check_pattern(rb_pattern)
                 File "/usr/lib/python3.7/site-packages/qiskit/ignis/verification/randomized_benchmarking/circuits.py", line 75, in check_pattern
                   pattern_flat.extend(pat)
               TypeError: 'int' object is not iterable

Indeed - the length_multiplier depended on the specific rb_pattern, so I removed this option.
I think that it should work now.
Thank you very much for the helpful feedback!

@mtreinish
Copy link
Member

mtreinish commented Jul 2, 2019

I just noticed your force push removed my commit (dcdd646 ) that added ignis to the asv conf. If you need to do another force push just make sure you include that commit in your local branch.

@mtreinish
Copy link
Member

I'm still getting local failures with this (basically the same one, just with a numpy int type instead of python's int):

               For parameters: 3, 10, [[0, 2], [1]], array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,
                       53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,
                      105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,
                      157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])
               Traceback (most recent call last):
                 File "/usr/lib/python3.7/site-packages/asv/benchmark.py", line 1039, in main_run_server
                   main_run(run_args)
                 File "/usr/lib/python3.7/site-packages/asv/benchmark.py", line 907, in main_run
                   skip = benchmark.do_setup()
                 File "/usr/lib/python3.7/site-packages/asv/benchmark.py", line 458, in do_setup
                   result = Benchmark.do_setup(self)
                 File "/usr/lib/python3.7/site-packages/asv/benchmark.py", line 390, in do_setup
                   setup(*self._current_params)
                 File "/home/mtreinish/git/qiskit/qiskit/test/benchmarks/randomized_benchmarking.py", line 73, in setup
                   seed=random_seed)
                 File "/home/mtreinish/git/qiskit/qiskit/test/benchmarks/randomized_benchmarking.py", line 50, in build_rb_circuit
                   rb_circs, _ = rb.randomized_benchmarking_seq(**rb_opts)
                 File "/home/mtreinish/git/qiskit/qiskit/.asv/env/a80b4bcff004e7d21aafa78a5932418b/lib/python3.7/site-packages/qiskit/ignis/verification/randomized_benchmarking/circuits.py", line 183, in randomized_benchmarking_seq
                   qlist_flat, n_q_max = check_pattern(rb_pattern)
                 File "/home/mtreinish/git/qiskit/qiskit/.asv/env/a80b4bcff004e7d21aafa78a5932418b/lib/python3.7/site-packages/qiskit/ignis/verification/randomized_benchmarking/circuits.py", line 75, in check_pattern
                   pattern_flat.extend(pat)
               TypeError: 'numpy.int64' object is not iterable

@ShellyGarion
Copy link
Member Author

I do not know why it's not working, as this option of parameters works well for me:

3 10 [[0, 2], [1]] [  1   5   9  13  17  21  25  29  33  37  41  45  49  53  57  61  65  69
  73  77  81  85  89  93  97 101 105 109 113 117 121 125 129 133 137 141
 145 149 153 157 161 165 169 173 177 181 185 189 193 197]
{'nseeds': 10, 'length_vector': array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,
        53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,
       105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,
       157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197]), 'rb_pattern': [[0, 2], [1]], 'length_multiplier': 1, 'seed_offset': 0, 'align_cliffs': False}

Several comments:

  • This was almost the last option, so perhaps we had too many options and ran out of memory.
    I noticed that I didn't use the nq parameter, so I removed it, and now there are fewer options.
  • Which version of ignis are you using? I am using the master version of ignis.
  • If it still fails, I would suggest to replace the params by:
params = ([1, 5, 10], [[[0]], [[0, 1]]],
              [np.arange(1, 200, 4), np.arange(1, 500, 10)])

namely, remove the option [[0, 2], [1]] for 1&2 qubits simultaneously.

@mtreinish
Copy link
Member

@ShellyGarion it was using the latest release as per what I added in: Qiskit/qiskit@b03f674 I did that because these benchmarks track terra so we want to minimize the differences between consecutive runs. If we track master that will potentially change things every time we merge a change to ignis (skewing the results). Is qiskit-community/qiskit-ignis#174 necessary for this benchmark to work?

As for being near the last option combination, that's just because I was lazy and didn't scroll up to the top, it wasn't a ram issue. Also every single parameter combination failed.

@mtreinish
Copy link
Member

I did some local debugging and pushed: Qiskit/qiskit@bb28c79 which fixes the issue I was hitting locally. We were previously passing the length vector as the RB pattern (and vice versa), that fixes the order.

@ShellyGarion
Copy link
Member Author

Thanks @mtreinish ! Is the code ready to be merged now?

@mtreinish
Copy link
Member

No, not quite yet; it's still failing. This time it looks like because the function to build the rb circuits is returning more than just circuits and the transpile calls fail because we're passing it not a circuit. I've got a fix pending to address this issue as well.

Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

Ok, I've pushed the last fix which makes it all run successfully locally: Qiskit/qiskit@b79159e


[ 75.00%] ··· ======== =============== ======================================================================================================================================================================================================================================================================================== ============
               nseeds     rb_pattern                                                                                                                                        length_vector                                                                                                                                                   
              -------- --------------- ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- ------------
                 1          [[0]]       array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,         53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,        105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,        157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])    312±4ms   
                 1          [[0]]       array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,        131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,        261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,        391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])    678±4ms   
                 1         [[0, 1]]     array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,         53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,        105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,        157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])    1.00±0s   
                 1         [[0, 1]]     array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,        131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,        261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,        391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])    2.31±0s   
                 1      [[0, 2], [1]]   array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,         53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,        105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,        157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])   1.26±0.01s 
                 1      [[0, 2], [1]]   array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,        131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,        261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,        391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])   3.09±0.05s 
                 5          [[0]]       array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,         53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,        105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,        157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])    303±6ms   
                 5          [[0]]       array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,        131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,        261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,        391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])    711±8ms   
                 5         [[0, 1]]     array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,         53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,        105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,        157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])    1.01±0s   
                 5         [[0, 1]]     array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,        131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,        261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,        391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])   2.37±0.02s 
                 5      [[0, 2], [1]]   array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,         53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,        105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,        157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])    1.30±0s   
                 5      [[0, 2], [1]]   array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,        131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,        261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,        391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])   3.05±0.01s 
                 10         [[0]]       array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,         53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,        105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,        157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])    313±2ms   
                 10         [[0]]       array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,        131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,        261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,        391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])    676±4ms   
                 10        [[0, 1]]     array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,         53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,        105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,        157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])    1.01±0s   
                 10        [[0, 1]]     array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,        131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,        261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,        391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])   2.39±0.01s 
                 10     [[0, 2], [1]]   array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,         53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,        105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,        157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])    1.27±0s   
                 10     [[0, 2], [1]]   array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,        131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,        261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,        391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])    3.14±0s   
              ======== =============== ======================================================================================================================================================================================================================================================================================== ============


[100.00%] ··· ...nchmarkingBenchmark.time_simulator_transpile                 ok
[100.00%] ··· ======== =============== ======================================================================================================================================================================================================================================================================================== ============
               nseeds     rb_pattern                                                                                                                                        length_vector                                                                                                                                                   
              -------- --------------- ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- ------------
                 1          [[0]]       array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,         53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,        105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,        157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])    197±10ms  
                 1          [[0]]       array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,        131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,        261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,        391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])    441±20ms  
                 1         [[0, 1]]     array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,         53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,        105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,        157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])    550±20ms  
                 1         [[0, 1]]     array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,        131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,        261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,        391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])   1.31±0.02s 
                 1      [[0, 2], [1]]   array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,         53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,        105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,        157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])    736±20ms  
                 1      [[0, 2], [1]]   array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,        131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,        261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,        391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])   1.73±0.02s 
                 5          [[0]]       array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,         53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,        105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,        157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])    210±8ms   
                 5          [[0]]       array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,        131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,        261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,        391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])    462±8ms   
                 5         [[0, 1]]     array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,         53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,        105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,        157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])    554±6ms   
                 5         [[0, 1]]     array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,        131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,        261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,        391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])   1.34±0.02s 
                 5      [[0, 2], [1]]   array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,         53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,        105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,        157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])    739±8ms   
                 5      [[0, 2], [1]]   array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,        131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,        261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,        391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])   1.71±0.01s 
                 10         [[0]]       array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,         53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,        105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,        157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])    224±7ms   
                 10         [[0]]       array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,        131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,        261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,        391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])    479±3ms   
                 10        [[0, 1]]     array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,         53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,        105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,        157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])    578±4ms   
                 10        [[0, 1]]     array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,        131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,        261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,        391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])   1.36±0.01s 
                 10     [[0, 2], [1]]   array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,         53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,        105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,        157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])   741±0.7ms  
                 10     [[0, 2], [1]]   array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,        131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,        261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,        391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])    1.67±0s   
              ======== =============== ======================================================================================================================================================================================================================================================================================== ============

However, it still can't merge yet it's way to slow to add to the benchmark suite. Running the full battery of benchmarks locally took 1.5 hours on a single terra commit (which is about 3x the time of the rest of the benchmark suite) to complete both benchmarks with the full 24 combination of input parameters. (it runs each benchmark multiple times) This is a non-starter since as we run the benchmarks over each merged commit (and my local desktop is faster than the baremetal machine we have provisioned for dedicated benchmarking). I think we need to do 2 things here first is switch the rb circuit build from the setup method to setup_cache, see: https://asv.readthedocs.io/en/stable/writing_benchmarks.html#setup-and-teardown-functions the second thing is remove all the parametrization from the benchmark and just use a single parameter. Both of these should hopefully mitigate the run time issue and enable us to run this on every commit.

@ShellyGarion
Copy link
Member Author

Thanks @mtreinish !
I think that perhaps we can reduce the number of combinations.
As @ajavadia wrote in Qiskit/qiskit#272 (comment):

I'm not sure varying the seed affects things much from a performance point of view (you can try).

Indeed, we see in the results that the parameter nseeds almost does not affect the time.
So, perhaps we can reduce the options for nseeds from [1, 5, 10] to [5].
This should hopefully reduce the time by 3x as needed.

@ShellyGarion
Copy link
Member Author

@mtreinish - I changed the parameters, so now we only have one option for nseeds (=[1]).
Could you please run the code again? If it's still slow I may reduce the length_vector parameter.

@mtreinish
Copy link
Member

@ShellyGarion you can easily run it locally too. You just need to have asv installed pip install asv (you might also need to run pip install virtualenv too) and then run asv run --bench randomized that will run just these benchmarks on a single commit to give you an idea of how fast it is.

That being said I ran it locally and it only took about 25mins to run. While that is a significant reduction I still feel it is too slow. A normal full benchmark run without these new benchmarks takes about 30-35min on my desktop so we'll be bumping the run time by a lot if we add that much time on top of it.

I also looked more into setup_cache and it is not an option for us here because it does not work for parameterized benchmarks.

This also raises a question for me about where we're actually spending all the time here. When you look at the benchmark results:

[ 75.00%] ··· ...markingBenchmark.time_ibmq_backend_transpile                 ok
[ 75.00%] ··· ======== =============== ======================================================================================================================================================================================================================================================================================== ============
               nseeds     rb_pattern                                                                                                                                        length_vector                                                                                                                                                   
              -------- --------------- ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- ------------
                 1          [[0]]       array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,         53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,        105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,        157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])    302±5ms   
                 1          [[0]]       array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,        131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,        261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,        391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])    667±10ms  
                 1         [[0, 1]]     array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,         53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,        105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,        157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])    1.00±0s   
                 1         [[0, 1]]     array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,        131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,        261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,        391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])   2.34±0.01s 
                 1      [[0, 2], [1]]   array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,         53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,        105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,        157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])    1.29±0s   
                 1      [[0, 2], [1]]   array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,        131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,        261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,        391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])    3.06±0s   
              ======== =============== ======================================================================================================================================================================================================================================================================================== ============

[100.00%] ··· ...nchmarkingBenchmark.time_simulator_transpile                 ok
[100.00%] ··· ======== =============== ======================================================================================================================================================================================================================================================================================== ============
               nseeds     rb_pattern                                                                                                                                        length_vector                                                                                                                                                   
              -------- --------------- ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- ------------
                 1          [[0]]       array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,         53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,        105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,        157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])    197±4ms   
                 1          [[0]]       array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,        131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,        261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,        391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])    441±4ms   
                 1         [[0, 1]]     array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,         53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,        105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,        157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])    531±10ms  
                 1         [[0, 1]]     array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,        131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,        261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,        391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])    1.30±0s   
                 1      [[0, 2], [1]]   array([  1,   5,   9,  13,  17,  21,  25,  29,  33,  37,  41,  45,  49,         53,  57,  61,  65,  69,  73,  77,  81,  85,  89,  93,  97, 101,        105, 109, 113, 117, 121, 125, 129, 133, 137, 141, 145, 149, 153,        157, 161, 165, 169, 173, 177, 181, 185, 189, 193, 197])    712±10ms  
                 1      [[0, 2], [1]]   array([  1,  11,  21,  31,  41,  51,  61,  71,  81,  91, 101, 111, 121,        131, 141, 151, 161, 171, 181, 191, 201, 211, 221, 231, 241, 251,        261, 271, 281, 291, 301, 311, 321, 331, 341, 351, 361, 371, 381,        391, 401, 411, 421, 431, 441, 451, 461, 471, 481, 491])   1.71±0.01s 
              ======== =============== ======================================================================================================================================================================================================================================================================================== ============

each transpile call is actually not that slow, taking in the worst case ~3 seconds even factoring in that asv runs each test multiple times it doesn't add up to 25mins (I did also just find a bug that I'll push a patch up for shortly which increases the number of circuits we transpile). It feels like we're spending most of the time building the circuits. But when I run it locally it's also relatively quick so I'm not sure what is taking so long

This commit fixes a number of issues with the benchmark. First it
removes single entry parameters from the parameter list, there's no
reason to make them parameters if we don't use multiple values. It also
fixes a small bug in the return from the build_rb_circuit() function
where we were only returning one sequence of circuits instead of the
whole set. It also adjusts how we use random values by properly setting
a seed for both circuit building and tranpile. The last thing it does is
for consistency it pins the ignis version to a single version and sets
the benchmark version number to match that. This way if/when we need to
bump the ignis version we use in the benchmarks we'll have to bump the
benchmark's version too to match. This way we can make sure we're
using the same ignis version for all benchmark results.
@mtreinish
Copy link
Member

I just pushed up: Qiskit/qiskit@31380d5 which fixes several issues I identified in the benchmarks. But, we still will want to reduce it to be a single length vector for time. Unless we can figure out what is causing it to take so long. Maybe we also want to time building the rb circuits not just the time to transpile them.

@ShellyGarion
Copy link
Member Author

This is strange. From my experience, usually generating the circuits is much faster than transpiling & running them. Perhaps you can only try the option:
length_vector = np.arange(1, 200, 4)
(maybe the other 500-length sequence is too long for the cache).

class RandomizedBenchmarkingBenchmark:
# parameters for RB (1&2 qubits):
params = ([[[0]], [[0, 1]], [[0, 2], [1]]],
[np.arange(1, 200, 4), np.arange(1, 500, 10)])
Copy link
Member

Choose a reason for hiding this comment

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

Instead of using the raw arrays as parameters (giving in the long lines in the asv output), maybe use a tuple of (start, stop, step) as a parameter and build the length vector in setup?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure, since sometimes we may like to use other sequences (say lengths that grow exponentially and not linearly)

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, maybe then a tuple of (sequence_type, sequence_args)? The parameters will end up being displayed in the web interface (see e.g. https://qiskit.github.io/qiskit/#circuit_construction.CircuitConstructionBench.time_circuit_extend ) and the full list would be quite unwieldy.

Copy link
Member Author

Choose a reason for hiding this comment

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

The last commit handles this, since there is only a single length_vector, so this parameter is not needed anymore.

@ajavadia
Copy link
Member

This is strange. From my experience, usually generating the circuits is much faster than transpiling & running them.

Yeah I think I have also seen most of the time spent in transpile, specifically the Unroller pass. @chriseclectic saw this recently too I think?

@kdk
Copy link
Member

kdk commented Jul 12, 2019

It's entirely possible the cost of circuit construction has increased recently, terra has had some regressions in its circuit building performance recently.

I tested a quick benchmark that only builds the circuits and it was comparable in time to the transpilation benchmarks.

· Running 3 total benchmarks (1 commits * 1 environments * 3 benchmarks)
[  0.00%] · For qiskit-terra commit 8a553fae <master>:
[  0.00%] ·· Benchmarking virtualenv-py3.5
[ 16.67%] ··· Running (randomized_benchmarking.RandomizedBenchmarkingBenchmark.time_ibmq_backend_transpile--).
[ 33.33%] ··· Running (randomized_benchmarking.RandomizedBenchmarkingBenchmark.time_rb_circuit_construction--).
[ 50.00%] ··· Running (randomized_benchmarking.RandomizedBenchmarkingBenchmark.time_simulator_transpile--).
[ 66.67%] ··· randomized_benchmarking.RandomizedBenchmarkingBenchmark.time_ibmq_backend_transpile                           1/6 failed
[ 66.67%] ··· =============== ============ ============
              --                    length_vector
              --------------- -------------------------
                 rb_pattern     (200, 4)    (500, 10)
              =============== ============ ============
                   [[0]]        393±4ms      892±10ms
                  [[0, 1]]     1.34±0.02s   3.63±0.03s
               [[0, 2], [1]]   2.18±0.02s     failed
              =============== ============ ============

[ 83.33%] ··· randomized_benchmarking.RandomizedBenchmarkingBenchmark.time_rb_circuit_construction                                     ok
[ 83.33%] ··· =============== ============ ============
              --                    length_vector
              --------------- -------------------------
                 rb_pattern     (200, 4)    (500, 10)
              =============== ============ ============
                   [[0]]        521±60ms    1.17±0.02s
                  [[0, 1]]     1.72±0.1s    4.29±0.06s
               [[0, 2], [1]]   2.38±0.02s   6.54±0.07s
              =============== ============ ============

[100.00%] ··· randomized_benchmarking.RandomizedBenchmarkingBenchmark.time_simulator_transpile                                      ok
[100.00%] ··· =============== ========== ===========
              --                  length_vector
              --------------- ----------------------
                 rb_pattern    (200, 4)   (500, 10)
              =============== ========== ===========
                   [[0]]       280±7ms     569±3ms
                  [[0, 1]]     705±2ms     1.69±0s
               [[0, 2], [1]]   920±10ms    2.25±0s
              =============== ========== ===========

@mtreinish
Copy link
Member

There definitely has been a large regression in the performance of circuit construction introduced by Qiskit/qiskit#2414: https://qiskit.github.io/qiskit/#circuit_construction.CircuitConstructionBench.time_circuit_extend?p-width=20&p-gates=2048&commits=79c44e56

@ShellyGarion
Copy link
Member Author

Is this PR ready to be merged?

@kdk
Copy link
Member

kdk commented Jul 16, 2019

Can you reduce it to a single vector (presumably arange(1, 500, 10))? We can re-introduce arange(1,200,4) after Qiskit/qiskit#2529 merges and circuit construction times drop.

This commit removes one of the two length vectors we were using for
constructing the RB circuits. This is done in the interest of time since
the RB benchmarking can be quite slow so this reduces the number of
input parameters we iterate over which decreases the total run time of
RB benchmarks. If in the future we improve the performance so that the
impact is minimal to the full asv run we can add back additional length
vectors for benchmarking.
@ShellyGarion
Copy link
Member Author

Thanks @mtreinish ! Is this PR ready to be merged now?

Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

This LGTM now I think (after I updated the license header). I have one inline comment, which is more about how much data we want to backfill after we merge this.


import numpy as np
import qiskit.ignis.verification.randomized_benchmarking as rb
from qiskit.providers.basicaer import QasmSimulatorPy
Copy link
Member

Choose a reason for hiding this comment

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

The only thing left that I see here is this line. Depending on how far back we want to backfill the benchmark data this will be the limiting factor since it was added in Qiskit/qiskit#1854 which is post 0.7.0. If we wanted data going back to 0.7 we'll have to handle that here.

@jaygambetta
Copy link
Member

@mtreinish is there a reason you did not merge after approving?

@mtreinish
Copy link
Member

@jaygambetta not really, there was an open question on about the length of history to build, but it's not a blocker. I just didn't merge before because I was just waiting on CI to finish and then forgot to come back and hit the merge button.

@mtreinish mtreinish merged commit 9d869cc into Qiskit:master Jul 21, 2019
@ShellyGarion
Copy link
Member Author

Thanks @mtreinish !

jakelishman pushed a commit to jakelishman/qiskit-terra that referenced this pull request Aug 1, 2023
…e#306)

* first version of benchmark/randomize_benchmarking.py

* fix lint error

* fixed a misprint

* updated benchmark test for RB

* fixed RB benchmark test following comment

* fix lint error

* Add qiskit-ignis to benchmark env

* removed nq from the parameters

* Correct setup() parameter order

* Only return circuits for transpilation

* nseeds = [1]

* Fix various issues with the benchmark

This commit fixes a number of issues with the benchmark. First it
removes single entry parameters from the parameter list, there's no
reason to make them parameters if we don't use multiple values. It also
fixes a small bug in the return from the build_rb_circuit() function
where we were only returning one sequence of circuits instead of the
whole set. It also adjusts how we use random values by properly setting
a seed for both circuit building and tranpile. The last thing it does is
for consistency it pins the ignis version to a single version and sets
the benchmark version number to match that. This way if/when we need to
bump the ignis version we use in the benchmarks we'll have to bump the
benchmark's version too to match. This way we can make sure we're
using the same ignis version for all benchmark results.

* Reduce the number of length vectors used to 1

This commit removes one of the two length vectors we were using for
constructing the RB circuits. This is done in the interest of time since
the RB benchmarking can be quite slow so this reduces the number of
input parameters we iterate over which decreases the total run time of
RB benchmarks. If in the future we improve the performance so that the
impact is minimal to the full asv run we can add back additional length
vectors for benchmarking.

* Update copyright header to use consistent format
jakelishman pushed a commit to jakelishman/qiskit-terra that referenced this pull request Aug 11, 2023
…e#306)

* first version of benchmark/randomize_benchmarking.py

* fix lint error

* fixed a misprint

* updated benchmark test for RB

* fixed RB benchmark test following comment

* fix lint error

* Add qiskit-ignis to benchmark env

* removed nq from the parameters

* Correct setup() parameter order

* Only return circuits for transpilation

* nseeds = [1]

* Fix various issues with the benchmark

This commit fixes a number of issues with the benchmark. First it
removes single entry parameters from the parameter list, there's no
reason to make them parameters if we don't use multiple values. It also
fixes a small bug in the return from the build_rb_circuit() function
where we were only returning one sequence of circuits instead of the
whole set. It also adjusts how we use random values by properly setting
a seed for both circuit building and tranpile. The last thing it does is
for consistency it pins the ignis version to a single version and sets
the benchmark version number to match that. This way if/when we need to
bump the ignis version we use in the benchmarks we'll have to bump the
benchmark's version too to match. This way we can make sure we're
using the same ignis version for all benchmark results.

* Reduce the number of length vectors used to 1

This commit removes one of the two length vectors we were using for
constructing the RB circuits. This is done in the interest of time since
the RB benchmarking can be quite slow so this reduces the number of
input parameters we iterate over which decreases the total run time of
RB benchmarks. If in the future we improve the performance so that the
impact is minimal to the full asv run we can add back additional length
vectors for benchmarking.

* Update copyright header to use consistent format
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants