[adlantz]

Issue #336 asks for improvements to the tfq.convert_to_tensor speed function. The majorirty of the run time occurs in the cirq serialization logic:

============================================================
Where does convert_to_tensor spend its time?
============================================================
  tensorflow_quantum 0.7.7 | cirq 1.5.0 | python 3.10.14
  serialize_circuit: deepcopy (baseline)  [git HEAD]
  best of 3 runs per phase

100 x 10q depth 20
  --------------------------------------------------------
  convert_to_tensor (total)                  397.4 ms   100.0%
  --------------------------------------------------------
  1. serialize_circuit  (cirq, Python)       392.4 ms    98.7%
  2. SerializeToString  (protobuf)             3.3 ms     0.8%
  3. remainder          (tensor packing)       1.7 ms     0.4%

100 x 20q depth 50
  --------------------------------------------------------
  convert_to_tensor (total)                 1913.2 ms   100.0%
  --------------------------------------------------------
  1. serialize_circuit  (cirq, Python)      1941.8 ms   101.5%
  2. SerializeToString  (protobuf)            18.9 ms     1.0%
  3. remainder          (tensor packing)     -47.6 ms    -2.5%

50 x 30q depth 100
  --------------------------------------------------------
  convert_to_tensor (total)                 2944.9 ms   100.0%
  --------------------------------------------------------
  1. serialize_circuit  (cirq, Python)      2938.0 ms    99.8%
  2. SerializeToString  (protobuf)            29.5 ms     1.0%
  3. remainder          (tensor packing)     -22.6 ms    -0.8%

100 x 20q depth 50 (+ctrl)
  --------------------------------------------------------
  convert_to_tensor (total)                 1960.2 ms   100.0%
  --------------------------------------------------------
  1. serialize_circuit  (cirq, Python)      1947.9 ms    99.4%
  2. SerializeToString  (protobuf)            19.7 ms     1.0%
  3. remainder          (tensor packing)      -7.4 ms    -0.4%

Phases are timed independently, so the remainder is a difference of minima. Small negatives are noise, read as ~0.

Inspecting serialize_circuit I noticed a quick win would be to not do a full deep copy of the input circuit, instead create a shallow copy with circuit = circuit_inp.copy(). The risk with this is that we may modify the input circuit (change in behavior). We can get away with this because moments are replaces rather than edited in place by the serializer, i.e

circuit[i] = cirq.Moment(
            new_ops[op.qubits] if op.qubits in new_ops else op for op in moment)

That being said, in the controlled operations demotion loop, we reference and mutate op.sub_operation which points to an attribute on the input circuit. Therefore, we make a copy there of each sub operation.

for op in controlled_ops:
    # Copy before tagging: `sub_operation` is a live reference into
    # the caller's circuit, and these attributes would leak onto it.
    tfq_compatible = copy.copy(op.sub_operation)
    tfq_compatible._tfq_control_qubits = op.controls
    tfq_compatible._tfq_control_values = op.control_values
    new_ops[op.qubits] = tfq_compatible

Overall the speedup is significant:

==================================================================
serialize_circuit: before vs after
==================================================================
  baseline  (HEAD):  deepcopy
  candidate (working tree): shallow copy
  cirq 1.5.0 | python 3.10.14 | Darwin arm64
  best of 5 alternating rounds

  correctness
    output identical      yes   (2b2bc6dd05e14f75...)
    input circuits clean  yes

  100 x 10q depth 20                378.5 ->    277.1 ms    1.37x
  500 x 10q depth 20               1895.1 ->   1363.3 ms    1.39x
  100 x 20q depth 50               1888.7 ->   1402.4 ms    1.35x
  50 x 30q depth 100               2841.5 ->   2121.3 ms    1.34x
  100 x 20q depth 50 (+ctrl)       1928.5 ->   1436.3 ms    1.34x

The downside is less peace of mind that the input circuit is not mutated, and any changes here in the future need to keep that in mind.

I added a test to ensure serialization does not mutate the input circuit.

[mhucka]

Thanks for this improvement and for the careful before/after numbers!

Here's a preliminary review. I still need to do some more testing, and will come back to this when I can.

[mhucka]

Possible additional performance improvement: if a moment does not have a controlled operation, a new Moment many not need to be created to replace it later below on lines 885-886. So, the following might be worth adding:

Suggested change

	]
	]
	if not controlled_ops:
	continue

[mhucka]

Might be worth adding to test_serialize_does_not_tag_input_circuit:

In addition to asserting self.assertFalse(hasattr(sub_op, '_tfq_control_qubits')) and self.assertFalse(hasattr(sub_op, '_tfq_control_values')), serializing cirq.Circuit(sub_op) afterward and asserting that

proto.circuit.moments[0].operations[0].args['control_qubits'].arg_value.string_value == ''

will verify observable serialization output even if internal attribute names change in the future.

[mhucka]

An additional point: the existing test checks hasattr(sub_op, '_tfq_control_qubits'). That catches the exact leak this PR guards against. But it is tied to a private attribute name, and it doesn't show why a leak matters. The real hazard: if a tagged sub-operation object is reused somewhere else uncontrolled, it later serializes as a controlled gate. That changes simulation results silently.

[mhucka]

@adlantz Thank you for the benchmarks in the PR description. Could you also attach the benchmark script?