Speed-up of AbelianGrouper.group_subops

[t-imamichi]

Summary

Faster version of AbelianGrouper.group_subops with numpy.
As a PoC, I implemented AbelianGrouper.group_subops_fast.
I know it is necessary to use commutes method, but it takes so long time to check commutes between all pairs of operators.

Details and comments

TPB based grouping can be implemented with numpy as tpb_grouped_weighted_pauli_operator.py does.
I implemented the same technique as AbelianGrouper.group_subops_fast.
I also tested a trick to reduce the overhead of Graph.add_edges_from by directly inserting elements to Graph._adj.

I compared them using pauli_grouping.py.
I used NH3JW.txt of https://arxiv.org/src/1701.08213v1/anc/TaperingPaperHamiltonians.rar
'original' is the result of AbelianGrouper.group_subops, 'fast1' is that of group_subops_fast without the networkx trick, and 'fast2' is that of group_subops_fast with the networkx trick.
The resulting groups are identical and 'fast2' is the fastest.

$ python pauli_grouping.py -i TaperingPaperHamiltonians/NH3/NH3JW.txt
$ diff3 original.txt fast1.txt fast2.txt
====
1:1c
  Elasped time: 190.92967315999977
2:1c
  Elasped time: 39.291700249000314
3:1c
  Elasped time: 21.408162898999763

[t-imamichi]

I don't think this build error is not caused by my change.

=========================
Failures during discovery
=========================
--- import errors ---
Failed to import test module: test.aqua.operators.test_pauli_expectation
Traceback (most recent call last):
  File "/opt/python/3.8.0/lib/python3.8/unittest/loader.py", line 436, in _find_test_path
    module = self._get_module_from_name(name)
  File "/opt/python/3.8.0/lib/python3.8/unittest/loader.py", line 377, in _get_module_from_name
    __import__(name)
  File "/home/travis/build/Qiskit/qiskit-aqua/test/aqua/operators/test_pauli_expectation.py", line 28, in <module>
    from qiskit import BasicAer, IBMQ
ImportError: cannot import name 'IBMQ' from 'qiskit' (/home/travis/virtualenv/python3.8.0/lib/python3.8/site-packages/qiskit/__init__.py)
Failed to import test module: test.aqua.test_vqe
Traceback (most recent call last):
  File "/opt/python/3.8.0/lib/python3.8/unittest/loader.py", line 436, in _find_test_path
    module = self._get_module_from_name(name)
  File "/opt/python/3.8.0/lib/python3.8/unittest/loader.py", line 377, in _get_module_from_name
    __import__(name)
  File "/home/travis/build/Qiskit/qiskit-aqua/test/aqua/test_vqe.py", line 21, in <module>
    from qiskit import BasicAer, QuantumCircuit, IBMQ
ImportError: cannot import name 'IBMQ' from 'qiskit' (/home/travis/virtualenv/python3.8.0/lib/python3.8/site-packages/qiskit/__init__.py)

[dongreenberg]

This is awesome!! Maybe we should check inside convert to see whether the summedop only contains PauliOps, and if that's the case, use the fast method, and otherwise use the .commutes method?

Alternatively, is there an elegant way to add the numpy speed directly into the PauliOp's .commutes method?

[t-imamichi]

@dongreenberg Thank you for your comments. It is a good idea to call the fast version only if the summedop only contains PualiOps. I will write a code to realize it and add unit tests.
It may be also possible to implement PualiOp's .commutes with numpy. I will try it too.

[t-imamichi]

PauliOp.commutes already uses numpy. The problem is that it computes the ndarray every time commutes is called; but the ndarrays do not change during all pair comparison. So the ndarray generation is the bottleneck.
https://github.com/Qiskit/qiskit-aqua/blob/master/qiskit/aqua/operators/primitive_ops/pauli_op.py#L248

So, I think a special pass for ListOp consisting of only PauliOp is the best approach.

[t-imamichi]

I added an option to control the pass (generic or fast) and update the test script

The result of H2O JW:

$ python pauli_grouping.py -i TaperingPaperHamiltonians/H2O/H2OJW.txt
$ diff3 original.txt fast.txt fast-nxtrick.txt
====
1:1c
  Elasped time: 21.976199786999132
2:1c
  Elasped time: 4.463234193000972
3:1c
  Elasped time: 2.1695586320001894

Unit tests are not completed yet.

[t-imamichi]

Thank you for your comments. I changed the default options and moved the unit tests to test_pauli_expectaion.

[mtreinish]

Fwiw, please feel free to open an issue against retworkx to add the coloring support. The first priority for it when I started was the terra's usage for the DAGCircuit and the transpiler. So outside of that it doesn't have many features yet. But, now that it is in place expanding retworkx to support other needs from a graph library is totally fair and definitely worth it. I've got an in-progress PR for undirected graphs open now Qiskit/rustworkx#68. Once that's merged adding coloring algorithms (especially the largest_first which is just a list of nodes sorted by degree) should be pretty straightforward.

[t-imamichi]

Thanks! It's a good news. When undirected graphs are merged, I want to try it.

[dongreenberg]

Sorry - I think I may have been unclear about something. I liked that you pull the AbelianGrouper tests into a separate file. Can we just move test_abelian_grouper1 2 and 3 into the new test_abelian_grouper file and out of test_pauli_expectation?

[t-imamichi]

Sorry. I will move the tests to a separate file.

[t-imamichi]

I moved abelian grouper tests to a separate file test_abelian_grouper.py and added a unit test with random pauli operators.

[dongreenberg]

Looks great, thanks Imamichi-san!!!!

[t-imamichi]

Thank you for fixing lint.

[dongreenberg]

@woodsp-ibm should I hold off on merging this until after the patch to stable today?

[woodsp-ibm]

@dongreenberg Yes, if we can hold off on this until we do the release, thanks.