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

ParametricQuantumCircuitに対して、add_gate_copy()した際にパラメータ共有しているポジションがずれないか? #192

Open forest1040 opened 2 years ago

forest1040 commented 2 years ago

ちょっと気になりました。 https://github.com/Qulacs-Osaka/qulacs-osaka/blob/dev/src/vqcsim/parametric_circuit.cpp#L136

ikanago commented 2 years ago

たしかにずれると思いますが,PrametricQuantumCircuit のインスタンスは LearningCircuit を介して操作することになっており,これが問題になるのは circuit: LearningCircuit に対して circuit._circuit.add_gate_copy() を呼び出した場合です. しかし,circuit._circuit: PrametricQuantumCircuit は private member なのでそのようなアクセスをするほうが悪いというふうに考えています(強制できないので微妙なところではありますが...).

forest1040 commented 2 years ago

merge_circuit()を呼び出すと内部的にadd_gate_copy()が呼ばれてしまうため、ちょっと気になりました。。

ikanago commented 2 years ago

ほんとですね,#186 で merge_circuit() を使う話が出てるので問題になりそうですね... パラメータ共有の実装を ParametricQuantumCircuit まで降ろして add_gate_copy() で共有されたパラメータがずれないように処理するのがいいかもしれません.

forest1040 commented 2 years ago

そうですね。。 そういえば、バックエンドがMPIQulacs等で分散した場合もある程度、大丈夫なように考慮しといた方がよいかも知れません。。 まだ、MPIQulacsの構成を理解していませんが。。

forest1040 commented 2 years ago

まぁバックエンドの分散までは、今時点では考えすぎな気もしてきました。。

ikanago commented 2 years ago

merge_circuit() が呼ぶのは add_gate_copy(const QuantumGateBase* gate) のほうなのでパラメータがずれたりはしなくて,LearningCircuit.merge_circuit() で適切に処理すれば問題ないかもしれないです

forest1040 commented 2 years ago

なるほど!確かに