Qulacs-Osaka / scikit-qulacs

scikit-qulacs is a library for quantum neural network. This library is based on qulacs and named after scikit-learn.
https://qulacs-osaka.github.io/scikit-qulacs/index.html
MIT License
21 stars 6 forks source link

add a function to merge circuit #186

Open herring101 opened 2 years ago

herring101 commented 2 years ago

a_circuit += b_circuit といった書き方で2つの回路をつなげる機能を作ろうと思います。(Qiskitにもこの機能は用意されていました)

QuantumCircuitクラスのmerge_circuitメソッドを用いることで代替可能ですが、より見やすくするために特殊メソッドを用いた上記の機能を追加しようと思います。

WATLE commented 2 years ago

これまでqulacsに演算子を用いた動きがなくて、いきなり+= を使うというのは、一貫性というか.. う~ん... ?

それと、もしやるなら、c_circuit = a_circuit + b_circuit という書き方も許可してほしい

WATLE commented 2 years ago

ん、scikit-qulacsに追加するんですか? qulacs-osakaに追加するもんだと思ってました

herring101 commented 2 years ago

ん、scikit-qulacsに追加するんですか? qulacs-osakaに追加するもんだと思ってました

そうですね、qulacs-osakaに追加してから、scikit-qulacsでも使えるようにしようと思ってました。qulacs-osakaでissueを立てた方がよかったですね...

これまでqulacsに演算子を用いた動きがなくて、いきなり+= を使うというのは、一貫性というか.. う~ん... ? それと、もしやるなら、c_circuit = a_circuit + b_circuit という書き方も許可してほしい

c_circuit = a_circuit + b_circuitという書き方もできるようにはしようと思ってます。一貫性に関してですが、別の書き方が許される程度なら大丈夫な気はするんですがどうでしょうか?

kosukemtr commented 2 years ago

 + じゃなくて append_circuit(足したい回路) みたいな関数を作るのではだめでしょうか

herring101 commented 2 years ago

確かに、その関数を作る方がよさそうですね。 早速やってみようと思います。

herring101 commented 2 years ago

merge_circuitをラップすればいいだけな気がしてきました。 そのままメソッド名もmerge_circuitでいいですかね。

herring101 commented 2 years ago

↑のissueでかかれているように問題がありそうです。 あまり理解できていないため、ある程度調べ終わるまで中断しておきます...

forest1040 commented 2 years ago

@herring101 もしかしたら、実装は、 @ikanago にやっていただいた方がよいかも知れませんね。。共有パラメータが絡みますので。 両名がよろしければですが。。なんでしたら、自分がやりますが。

ikanago commented 2 years ago

↑の issue を実装して qulacs-osaka の新バージョンがリリースされるまで待ってもらうことになりそうです

herring101 commented 2 years ago

ありがとうございます:bow:

ikanago commented 2 years ago

192 ですが,実は問題なさそうです.

少し整理します: PrametricQuantumCircuit::merge_circuit()PrametricQuantumCircuit::add_gate_copy(const QuantumGateBase* gate, UINT index) を呼んでいると思っていました. このメソッドはコピーした gateindex の位置に挿入して index より後ろにもともと存在したゲートの位置を1つ後ろにずらします. このとき LearningCircuit 内の共有されたパラメータの位置がずれそうだという問題がありました. しかしよくよく見ると,PrametricQuantumCircuit::merge_circuit() が呼んでいるのは PrametricQuantumCircuit::add_gate_copy(const QuantumGateBase* gate) で,これは単にコピーしたゲートを append するだけなので既存のゲートには影響しません.

よって,LearningCircuit.merge_circuit() 側の実装で適切に処理するだけで問題ないと思われます. 具体的には,3つのゲートをもつ circuit1: LearningCircuit に 4つのゲートをもつ circuit2: LearningCircuit をマージすることを考えると,circuit2 がもつパラメータの positions_in_circuit などに3を足した上で circuit1 に append すればよさそうです.

ikanago commented 2 years ago

LearningCircuit の実装を把握している人を増やした方がいいので @herring101 にやってもらえないかなと思うんですが,どうですか? 自分以外の人が読んでわかりやすいコメントを書けているかも気になるので...

forest1040 commented 2 years ago

@ikanago @herring101 はい!よいと思います。 いかがでしょうか? > herring101

herring101 commented 2 years ago

やります!

forest1040 commented 2 years ago

よろしくお願いします!なんかややこしくして、すみませんでした!

ikanago commented 2 years ago

最低限こんなテストが必要そうです.他にもやるべきテストがあったらどんどん追加しておいてください.

def test_merge_circuit_with_shared_parameter():
    circuit1 = LearningCircuit(2)
    circuit1.add_parametric_RX_gate(0, 0.0)
    circuit1.add_parametric_RY_gate(1, 0.0, share_with=0)
    circuit1.add_parametric_RZ_gate(0, 0.0)

    circuit2 = LearningCircuit(2)
    circuit2.add_parametric_RZ_gate(0, 0.0)
    circuit2.add_parametric_RY_gate(1, 0.0)
    circuit2.add_parametric_RZ_gate(0, 0.0, shared_with=1)
    circuit2.add_parametric_RX_gate(0, 0.0, shared_with=1)

    circuit1.merge_circuit(circuit2)
    assert circuit1._learning_parameter_list[0] == _LearningParameter([0, 1])
    assert circuit1._learning_parameter_list[3] == _LearningParameter([5, 6, 7])

def test_merge_circuit_with_shared_input_parameter():
    # 上のテストを input parameter を含む回路でやる
    ...

def test_invalid_merge_of_circuits_with_different_qubits():
    circuit1 = LearningCircuit(2)
    circuit2 = LearningCircuit(3)
    # c.f. https://docs.pytest.org/en/7.1.x/how-to/assert.html?highlight=exception#assertions-about-expected-exceptions
    with pytest.raises(RuntimeError):
        circuit1.merge_circuit(circuit2)
herring101 commented 2 years ago

かなり期間が空いてしまい申し訳ありません... 1つ問題点があったため報告しておきます。

self._circuit.merge_circuit(other._circuit)を行った後、circuit2のpositions_in_circuit,parameter_idの更新を行ってcircuit1の_learning_parameter_listにappendするようなmerge_circuitメソッドを作成しました。

そして以下のコードを実行しました。

circuit1 = LearningCircuit(2)
circuit1.add_parametric_RX_gate(0, 0.0)
circuit1.add_parametric_RY_gate(1, 0.0, share_with=0)
circuit1.add_parametric_RZ_gate(0, 0.0)

circuit2 = LearningCircuit(2)
circuit2.add_parametric_RZ_gate(0, 0.0)
circuit2.add_parametric_RY_gate(1, 0.0)
circuit2.add_parametric_RZ_gate(0, 0.0, share_with=1)
circuit2.add_parametric_RX_gate(0, 0.0, share_with=1)

print(circuit1._new_parameter_position())
circuit1.merge_circuit(circuit2)
print(circuit1._new_parameter_position())

出力

3
3

merge後にcircuit2の4つのゲート分を足した7という出力となってほしいのですが このようにmerge_circuitではparametric gateの個数の更新が行われません

merge_circuit自体がParametricQuantumCircuitの_parametric_gate_listの更新を行わないため、新たにParametricQuantumCircuit用のmerge_circuitを作る必要があるように思いました。

ikanago commented 2 years ago

どこかのブランチに実装を push してもらってもいいですか?

herring101 commented 2 years ago

186-merge-circuit にpushしました。 テスト用に作ってるnotebookは後で消しておきます

forest1040 commented 2 years ago

せっかくなので、次回の @herring101 所定労働時間に705で一緒にC++修正しちゃおうかと思います。cc @ikanago

ikanago commented 2 years ago

たしかに QuantumCircuitadd_gate_copy してるだけなので _parametric_gate_list が更新されないですね... 気づいたのすごいです 👍