Speed up cirq import by removing scipy.stats import #4041
Conversation
There was a problem hiding this comment.
Looks like this is used in von_neumann_entropy below, so not sure that we can remove the import: https://github.com/quantumlib/Cirq/blob/master/cirq-core/cirq/qis/measures.py#L275. Tests pass, but I don't know if we have coverage of that particular case in von_neumann_entropy.
Nice find! I guess we could still move the import into the von_neumann_entropy function and add a comment that this is a local import for import performance reasons? |
We do have coverage, at least I checked by printing debug right before the statement then failing a test and seeing that the print showed up. So yeah strange let me investigate. |
|
Oh lol, the test imports scipy.stats. |
|
OK, made the import delayed in the function. Alternatively we could just calculate the entropy using numpy, the only case you need to be careful about are the 0 * log(0) terms. I also updated the test to not use scipy.stats and just calculate the entropy directly. |
| np.testing.assert_allclose(cirq.von_neumann_entropy(mat), scipy.stats.entropy(probs, base=2)) | ||
|
|
||
| np.testing.assert_allclose( | ||
| cirq.von_neumann_entropy(mat), -np.sum(probs * np.log(probs) / np.log(2)) |
There was a problem hiding this comment.
nit: can use np.log2
| cirq.von_neumann_entropy(mat), -np.sum(probs * np.log(probs) / np.log(2)) | |
| cirq.von_neumann_entropy(mat), -np.sum(probs * np.log2(probs)) |
maffoo
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
CirqBot
removed
automerge
This speeds up import on my box by around 13%. Before:  After: 
This speeds up import on my box by around 13%.
Before:
After: