search

Qulacs-Osaka / qulacs-osaka

Development branch of qulacs at Osaka Univ
MIT License
13 stars 6 forks source link

Use pointer as a parameter #365

Closed KowerKoint closed 2 years ago

KowerKoint commented 2 years ago

345

ParametricGateを管理するParametricCircuitですが、パラメータ本体はゲートがそれぞれ独立に保持していて、同時に変える場合は一つずつ直接変更するというのがおかしいと思ったのでSingleParameterというパラメータのクラスを作ってそれをゲートではなくサーキットに持たせ、ParametricGateにはSingleParameterへのポインタをもたせる、という趣旨の変更です。これは大きな仕様変更であり、現状vqcsimのいくつかのライブラリを破壊してしまっているので細かな設計を相談の上決定したいです。

KowerKoint commented 2 years ago

暫定の変更に対する説明

SingleParameterについて

まずSingleParameter(parametric_gate.hpp)には

が含まれ、これらのメンバを参照したり書き換えたりするためのいくつかのメソッドが定義されています。

_idはgate_indicesはメソッドを使っても自由に書き換えることができ内容になっていますが、ParametricQuantumCircuitからの呼び出しなどで書き換えることは多いためこれらのメソッドは追加・変更する可能性があります。(SingleParameterをユーザーが直接触ることは基本的に想定していないためstructにしてすべてpublicにしたほうが楽かもしれません。)

ParametricGateの外部とのやりとりの削除について

次の大きな変更として、parametric_gate_factoryの削除、各ParametricGateのcopy()の削除(ただしこれは親クラスの純粋仮想関数であるためNotImplementedExceptionを出すだけ)があります。

パラメータはポインタとなりましたが、外でParametricGateを作る場合そのパラメータの実体は回路の外に置く必要があって管理できないためgate::ParametricRX()等のfactory系関数は削除しました。

同様に、ParametricGateがcopyされると回路内のパラメータへのポインタが無意味に回路外に持ち出されることになってややこしい(ダブルフリーなどの恐れがある)ためcopy()を未定義にしました。

ただし親クラスQuantumGateがgate_listをpublicにしているためユーザーは容易にParametricGateを外部に持ち出すことができるし、流石にこれを防ぐと不便すぎるためこの辺は妥協することになっています。

複数の回路で同じパラメータを共有するなどの需要があれば別の設計をする必要があるのでそこも考えどころです。

angle_funcについて

複数のパラメータを共有してもそれを「等しい回転角」など複数のゲートにおいて全く同じ解釈をするのは自由度が低いため、「パラメータから回転角への関数」angle_funcを各ゲートに持たせて、そのゲートを適用する時にパラメータから回転角へ変換してから適用するという機能を追加しています。 これによって「同じパラメータpを共有してるけどRYゲートAではp回転、別のRYゲートBでは-p回転」のような操作ができるようになります。 デフォルトでは恒等写像identity_mapが入っています。

scikit-qulacsのLearningCircuitにあるパラメータ共有機能では共有したゲートはすべて等しい回転角にしか設定できていないが成り立っているため不要なら削除します。 (少しだけ話は違うんですがMultiQubitPauliRotationGateも各パウリ演算子が全く等しい回転角しか持たないのでそういう需要しかないのか疑問です…)

add_parametric_XXX_gate()について

ParametricQuantumCircuitのadd_parametric_XXX_gate()メソッドについて、

の2種類にオーバーロードされました。これらは引数の型が似ていたりangle_funcにデフォルト値を与える都合でやや非自明な引数の並びになってしまっています。 後者の方は別名のメソッドにしたほうがいいかもしれません。

また、このメソッドではadd_gate()のように回路の中途に入れるオプションがまだ実装できていません。 個人的には実装しなくてもいいと思っていますが、これを実装するかどうかは下の「ゲートの挿入・削除について」に関連しています。

ゲートの挿入・削除について

そもそも量子回路の途中にゲートを挿入したり途中のゲートを削除したりする需要があるのかという疑問があって個人的にはいっそこれをすべて削除してしまいたいと思っています。（先頭の挿入・削除は需要あるかも…）

特にParametricGateは複数の箇所にゲートのインデックスが使われていて挿入・削除のたびにこれを管理するのが大変なのでやめたいです。

逆に挿入・削除機能を削除すると大変なのがcircuit_optimizerの実装です。optimizerをParametricCircuitに対応させたままにするには結局似たようなインデックス管理を実装する必要がでてきてしまいます…

仕様変更された関数について

パラメータの管理方法が変わったのでParametricQuantumCircuitのUIとしてのメソッドもそのままの仕様ではいられなくなってしまっています。いかに変更点を示します。

メソッド 変更点
copy() ParametricGateは直接コピーせずパラメータをすべてコピーして同義の回路を作成するよう変更
get_parameter_count() これまではParametricGateの数を返していたが、名前通りパラメータの数を返すようになっている。パラメータはID通りの位置に配置しないとバグるのでゲート削除で主が居なくなったパラメータも残り続けることになり、カウントされる

ParametricGateの全体での位置を追加した順に保持する_parametric_gate_positionや位置ではなくゲートを保持する_parametric_gate_listといったリスト、そのgetter/setterの役割をするメソッドは新しい思想では大した意味を持たない(パラメータのIDと_parametric_gate_positionのインデックスは対応しなくなっているのでややこしい)ので個人的には削除したいですがこれ以上互換性を破壊するのも微妙という感じです。 逆に各パラメータの参照されているゲートのインデックスのリスト(すなわちSingleParameter::_gate_indices)などをgetするメソッドなど新たにほしいです。

vqcsimの他のライブラリとの関係について

syntax errorになるような明らかなものに関しては新しいメソッドに修正しましたがGradCalculatorなどでParametricGateのcopyを使っていたり書き直さないといけない部分がありそうです(今テスト通ってない)。まだ読めていない/理解していないコードが関連しているので随時直したいです…

テストについて

前のテストを適当に修正して使ってますがパラメータ共有機能などいろいろテストを増やす必要があるので仕様がちゃんと決まったら書きます。

forest1040 commented 2 years ago

結局の所、vqe と circuit_optimizer がキモという所でしょうか。。 vqe は、実際に使っている方に聞いてみたいと思います。 ある程度使えるようになったら、そのブランチでvqeのサンプルをやってみて、他の方に聞いてみようと思います。 circuit_optimizer ちょっと自分も見てみます。

Hiroya-W commented 2 years ago

やりたいことは大体理解しました。 パラメータをポインタで持たせることで、パラメータを共有できるようにする方針は良さそうだと思っています。この変更がこの先の修正でどこまで影響しそうかは、まだイメージがついていないです...。

ParametricGateの外部とのやりとりの削除について

factory関数やcopy関数の削除は、その通りかな、と思っています。

複数の回路で同じパラメータを共有するなどの需要

のところは、僕は需要があるかどうかはわかっていません。

angle_funcについて これによって「同じパラメータpを共有してるけどRYゲートAではp回転、別のRYゲートBでは-p回転」のような操作ができるようになります。

この機能、面白いですね！パラメータの共有が出来るようになると、この機能が活用できそうです。ただ、懸念されているように、

等しい回転角しか持たないのでそういう需要しかないのか疑問です

このような共有の仕方しかしないのであれば、コードをあえて複雑にする必要も無いので、どうするかは考えたいですね。この部分も僕は需要があるかどうかわかっていません。

add_parametric_XXX_gate()について これらは引数の型が似ていたりangle_funcにデフォルト値を与える都合でやや非自明な引数の並びになってしまっています。 後者の方は別名のメソッドにしたほうがいいかもしれません。

第2引数がUINTかdoubleでメソッドを見分けるのは、なかなか厳しいですね...。別名のメソッドに分ける方が良さそうです。

仕様変更された関数について

あるゲートのパラメータを変えたいと思った時に、そのゲートのindexとパラメータで変更出来ていたところが、今の実装であれば、そのゲートが持つパラメータのIDを取ってくる必要が出てくる、という理解であっていますか？ ゲートを指定して、そのパラメータを変えられた方が分かりやすいかも、と思っています。（そのゲートのパラメータを変えたいだけなのに、そのゲートが持つパラメータのIDを意識しないといけなくなる、パラメータを共有することが無い場合は、パラメータIDをユーザに意識させたくない、というのが言いたいのですが、文字で伝わっていますか...） scikit-qulacsのLearningCircuitでは、パラメータを直接変えれたら嬉しいのは良く分かるので、ここも、どう使っているか？を聞いた方が良さそうな気もしますね。

KowerKoint commented 2 years ago

レビューコメントありがとうございます。

別名のメソッドに分ける方が良さそうです

これはやっぱり流石にそうなので変えておきます。 個人的には以前のParametricCircuitとは異なる設計であることを意識してadd_parametric_XXX_gate_new_parameterとadd_parametric_XXX_gate_share_parameterに分けて明確化したいですが、既存の使い方にある程度の互換性を持たせるために前者はadd_parametric_XXX_gateのままにしておくべきなのかわかりません。

あるゲートのパラメータを変えたいと思った時に、そのゲートのindexとパラメータで変更出来ていたところが、今の実装であれば、そのゲートが持つパラメータのIDを取ってくる必要が出てくる、という理解であっていますか？

以前は_parametric_gate_positionのインデックス(つまり削除されていないParametricGateのうち何番目か)で指定していて、これはこれで使いにくいと思ってます(ゲートやゲートのインデックスからparametric_gate_positionのインデックスを得る方法がない)。 この変更ではパラメータのIDになってます。removeが行われない上で共有機能を使わなければこれまでと同じ値になりますが、IDを直接触らせるならーザーには全くの別物であることを知らせる必要があります。

ゲートを指定して、そのパラメータを変えられた方が分かりやすいかも

これよさそうです。ゲートからパラメータIDを得るメソッドを用意しつつ、パラメータIDまたはゲートで指定できるようにオーバーロードするといいのかなと思いました。

KowerKoint commented 2 years ago

パラメータIDの代わりにゲートを指定するの、そのParametricGateが本当に回路に含まれているかの検証を諦めるかParametricGateの集合としてのHashSetをわざわざ用意することになって微妙な気がしてきました…

forest1040 commented 2 years ago

パラメータIDの件、cirqの場合はシンボルを指定する感じのインターフェースのようです。 https://www.tensorflow.org/quantum/tutorials/qcnn?hl=ja

def one_qubit_unitary(bit, symbols):
    """Make a Cirq circuit enacting a rotation of the bloch sphere about the X,
    Y and Z axis, that depends on the values in `symbols`.
    """
    return cirq.Circuit(
        cirq.X(bit)**symbols[0],
        cirq.Y(bit)**symbols[1],
        cirq.Z(bit)**symbols[2])

cirqの場合、回転ゲートに対して、乗数でパラメータ指定するようです。 そのパラメータをシンボルで指定できます。

SVGCircuit(one_qubit_unitary(cirq.GridQubit(0, 0), sympy.symbols('x0:3')))

この例では、SymPyでシンボルを作っています。

forest1040 commented 2 years ago

ParameterSetクラスをシングルトンで生成して、パラメータ管理はそこに任せるという感じですかね。。 良さそうですね。

QuantumGateなのにパラメータを内部で使ったりできてしまう

まぁ、これは実装者が注意するとして。。

パラメータの解放タイミングがわかりずらい

パラメータの減はそんなにないでしょうから、ParameterSetクラスが消えるタイミングで一斉削除ですかね。。

kodack64 commented 2 years ago

遅くになってからのコメントとなりすいません。修正ありがとうございます。現状はパラメータ更新がそもそも結構遅めのルーチンになっていると思うので、どうあれ修正していただけると幸いです。parametric quantum circuit関係については私は専門家ではないのとメリットデメリットあると思うのであまり強い意見は無いので、コメントを踏まえて採用するしないを決めてもらってokなのですが、後述するようにゲートの挿入/削除については出版論文での押しポイントや、今後の速度最適化のことを考えたときにちょっと慎重になってほしいという事情があります。

パラメータのポインタについて

生のポインタを使うのは参照先の変数が生きているか確認する手段が無かったりするのと、parametric quantum gateに対してcopyを使えなくするのは基底クラスにキャストされたときcopy可能かよくわからなくて大変だと思うので、パラメータ共有化が目的であれば、[パラメータ名 - 変数名]の辞書をParametricQuantumCircuit/Gateが受け取ってパラメータを刷新する機能を持たせる方が良いかなと思いました。つまり、ParametricQuantumCircuitクラスとParametricQuantumGateクラスに、update_configuration_with_parameter_map()みたいな関数を追加して、回転角を更新させるといったような感じです。 （ほぼkotamanegiさんの案と同じですが、私はparameter setはstd::mapそのままでも良いんじゃないかなと思っているのと、パラメータの更新は、update quantum stateよりかなり低頻度でしか行われないので、update quantum stateとupdate configurationは関数として分けたほうが良いんじゃないかな、と思いました。）

上記の方が良いかなという動機としては下記があります。

apply_func

kotamanegiさんも書いているようにシリアライズが困難になってしまうという点もあるのですが、同じぐらい気にしているのが勾配計算(GradientCalculator関係)との整合性で、パラメータとの間に関数が入るとその自動微分みたいなものが必要になってしまうんじゃないかなと思いました。このためこの機能についてはとりあえず今回のPRでは分けておいて、GradCalculator関係の実装と相談する方が良いのではないかなと思います。

ゲートの挿入削除

ゲートの挿入削除なのですが、これは上でもmentionされている通りCircuitOptimizerクラスでかなり使われている関数です。パラメータが参照されているゲートを添え字のリストで持とうとすると、この関数のせいで色々なものが大変になるというのはその通りであり、回路のゲートは典型的にそれほど重くないので毎回回路を作り直しても別にそんなに負担じゃなさそうというのも一理あるのですが、これは下記の理由から少なくとも基底クラスからいきなり外すのは一旦待ってほしいと考えています。

ベンチマーク上で本当に遅くなるのかはちょっと自信が無いのですが、実質的に遅くならなさそうなら削除してもokなのと、parametric quantum circuitクラスでは管理コストが高いためこのクラスでは挿入しようとしたexceptionを送出して実行できない旨をユーザに伝えるようにするなどは問題ないです。

MultiQubitPauliRotationGate

(少しだけ話は違うんですがMultiQubitPauliRotationGateも各パウリ演算子が全く等しい回転角しか持たないのでそういう需要しかないのか疑問です…)

このゲートは「複数のパウリ演算子の和で回転するゲート」ではなく、「複数の量子ビットに作用する単一のパウリ演算子で回転するゲート」なので、定義上回転角は一個しかないはずです。多分想像しているのは exp( i a_1 X_1 + i a_2 X_2) みたいなユニタリだと思いますが、このゲートがやっているのは exp( i a_1 X_1 X_2) みたいなゲートです。

kotamanegi commented 2 years ago

（ほぼkotamanegiさんの案と同じですが、私はparameter setはstd::mapそのままでも良いんじゃないかなと思っているのと、パラメータの更新は、update quantum stateよりかなり低頻度でしか行われないので、update quantum stateとupdate configurationは関数として分けたほうが良いんじゃないかな、と思いました。）

parameter setがstd::mapでいいというのは、少なくとも初期の実装時には良いと思います。 少し複雑な最適化や処理実行前のチェックなどを行うならば分離の必要があるかもしれませんが、初手の実装ではstd::mapで必要十分だと予想してます。

parameter setはupdate_quantum_state()を呼び出す時に状態ベクトルのポインタと同時に渡されることを想定していました。 ゲートにパラメータをもたせるのではなくて、実行時にパラメータを受け取るというお気持ちです。 こうすれば、copyされたゲートは同じ意味を持ちますからcopy時の問題も発生しないかと思います。

ゲートの挿入削除

緩和策に賛成で、ParametricQuantumCircuitに対して挿入削除を実行しようとしたら例外が発生する、という仕様が無難だと思います。

KowerKoint commented 2 years ago

コメントありがとうございます。 update_quantum_state()時にパラメータを持たせるという設計が確かに利便性が高そうでコピーもできるので利便性が高そうです。 この方針だとParametricGateのParametricCircuit上でのインデックスのリストなどを保つ必要がないのでゲートの挿入・削除もあまりややこしくなくなるかもしれません。できそうなら実装してみます。

KowerKoint commented 2 years ago

_parametric_gate_listとparametric_gate_position、使い所がわかりません…

forest1040 commented 2 years ago

_parametric_gate_listとparametric_gate_position、使い所がわかりません…

@herring101 が詳しいかも。

KowerKoint commented 2 years ago

forest1040 さんと相談をして、QuantumCircuitにある従来のUINTでのパラメータ指定方法を残すべくindex_ をプレフィックスにつけたIDで対応してみました。 ただしこのindexはサーキット上で連番で生成してるので、factoryから生成する方法を復活させることはできていません。

KowerKoint commented 2 years ago

それから、これは再設計前からなんですが get_parameter_count()が本来の意味とずれたものになっています… ゲートが削除されてもパラメータは削除されない仕様になったので余計におかしくなりましたが、互換性を失ってでも本来の意味に治すべきなんでしょうか。

KowerKoint commented 2 years ago

LearningCircuitでもやってた(気づいてませんでした…)のでangle_funcの変わりにパラメータの定数倍(parameter_coef)だけ実装しました。

KowerKoint commented 2 years ago

ご意見を受けてかなり作り直したので再度変更点をまとめます。

新仕様まとめ

ParameterKey, ParameterSetについて

パラメータをわかりやすく管理するため、パラメータのIDを文字列で管理できるようにしました。実体はstd::stringですが、変更の可能性や役割の明確化のためParameterKeyに型エイリアスしました。整数と違って長さによってはパフォーマンスに影響しうるのでできるだけ参照渡しにしました。 Keyはゲートに持たせ、パラメータのポインタを持たせるのはやめた(使うときは毎回引数で受け取る)ので、ゲートをコピーしたり外からゲートを持ち込んだりしてもサーキットのParameterSetにちゃんとその名前のパラメータがあれば問題は起きなくなりました。

パラメータはParameterSet(実体は`std::map<ParameterKey, double>)としてサーキットに持たせられるようになりました。 ParameterSetはゲート追加時に新規作成することもできますが、直接パラメータを作成したり削除したりキーの存在確認をしたりするメソッドをサーキットにもたせました。

add_parametric_XXX_gate_yyy_parameter() について

新たなパラメータを作成しつつそれをもたせる add_XXX_gate_new_parameter()と、既存のパラメータのIDに結びつけてそのパラメータをもたせる add_XXX_gate_share_parameter()に分けました。IDが文字列になったのでオーバーロードでもそれほど間違えにくくはなりましたが、メソッド名は分ける方針を選びました。

既存のadd_parametric_XXX_gate()について

いきなりadd_patametric_XXX_gate()を削除すると多数のコードが死んでしまうため、index_をプレフィックスに決めた連番のパラメータを作成しつつ追加するメソッドとして実装し直しました。

ParametricQuantumCircuit::copy()について

これまでのコードだと削除や途中挿入によって単調増加でなくなったparametric_gate_positionもコピーをすると単調増加に振り直されるような変な実装だったのでついでに実装し直しました。線形探索のstd::find()を使わずに済んだので計算量的にもO(COUNT*(COUNT+GATE_COPY_COST))からO(COUNT*GATE_COPY_COST)になったので途中挿入だらけの小さなゲートが大量に入っているようなケースについて詰まらなくなりました。

parameter_coefについて

angle_funcは複数の事情によってやめましたが、パラメータの定数倍を角度にするのはscikit-qulacsのためにも必要そうだったので倍率をゲートに持たせるようにしました。コンストラクタのデフォルト引数を1.0にしています。

どうしても破壊的な変更になったこと

ParametricQuantumCircuit::get_parameter_count()

これまでから_parametric_gate_list.size()を返していて、直接的な処理にはなっていません。 以前はゲートにパラメータをもたせてたいたのでパラメトリックゲート数=パラメータ数が確約されていて、ゲート削除時にパラメータの数も当然減っていましたが、今回の実装ではパラメータを別で持っていて、create_parameter()で作ったパラメータがなくなっていたら不自然だと思ったためゲートを削除してもパラメータは削除しない方針で作っています。 サーキット内の各ParameterKeyについてそれを持つゲートの個数(もしくはインデックスのリスト)を別で持つことにすればパラメータに依存するゲートがなくなったときに削除することも可能です。削除したほうがいいと思うなら意見ください。

この違いで問題になるのがget_parameter_count()をパラメトリックゲートの個数を返すものとして使ってるコードがあると壊れる可能性があることです。 get_parametric_gate_position()があるのにその引数の上限(パラメトリックゲートの個数)を得る特定のメソッドがなかったためこの用途で使ってる方がいないとは言えないです。

ParametricGateのupdate_quantum_state(),set_matrix()

パラメータを外部から引数で与えるようになったので与えない方法(従来の仮想関数のオーバーライド)だと当然計算不能なためexceptionを投げます。 ユーザーには単体のゲートに対して使うときParametricじゃない回転ゲートを必ず使ってほしい。

ParametricGateのgate_factory

パラメータIDなしにはParametricGateが使えなくなったのでgate::ParametricXX()の引数が変わっちゃいました。 これが一番でかくて、テストコードもあれこれ書き直さなければならなくなる場所です。

Exception

例外が追加されたり変更されたりしたのでC++のままtry...catchとか使ってる人がいたら挙動が変わっていまいます(流石にこれは気にしなくて良さそう)

テストについて

互換性が維持できてるか確認したいので一旦devブランチのテストに上書きし直しました。破壊的変更の話し合いがちゃんと済んだら書き直します。

Hiroya-W commented 2 years ago

ParameterKey, ParameterSet

良い方向性な気がします。ゲートを指定して、パラメータを変更したいかも？と思っていたのですが、パラメータを共有できる概念が生まれる以上、この方がいい気がしますね。 整数型よりもIDは文字列型の方が可読性も良さそうです。

add_parametric_XXX_gate()、add_parametric_XXX_gate_yyy_parameter()について

既存のadd_parametric_XXX_gate()もインターフェースは変わらず使えるようになっていて、別のメソッド名で作る方針は、いい気がします。

ParametricQuantumCircuit::get_parameter_count()

get_parameterkey_count(), get_parametric_gate_count()に分けるとかですかね...？同じ名前で残っていて壊れるのはありそうですね。

KowerKoint commented 2 years ago

get_parameter_count()がゲートの数っぽく捉えられてしまうことがやはりおおそうだったので削除してget_parametric_gate_count()とget_parameter_id_count()に分けました。

それから各処理にParameterSetを渡す以上、まるごとParameterSetをget/setできたほうが嬉しそうだったのでget_parameter_set()(参照ではなくコピーの受取)とset_parameter_set()を用意しました。ついでにコンストラクタにもParameterSetを渡してその場で構築できるようにしました。

GradCalculatorを新しいParameterSetに対応させてみました。この計算の意味に明るくないのでこの修正で合ってるかどうか確認がほしいです。

KowerKoint commented 2 years ago

get_parameter_count()、旧来の使い方でParametricQuantumCircuitを扱う人限定になるので、_next_parameter_indexを返すのが平和な気がしてきました。 そもそもindex_つき(旧メソッドで勝手につけられた名前)のパラメータとそうでないパラメータが混在すると怖いので例外を出しちゃってもいいかもですね…

修正するなら全部自分で文字列作ってID指定してほしいし、しないなら中途半端に新方式使わずに全部旧来のメソッドで1:1対応でやってくれるとバグりにくそうで助かります。

KowerKoint commented 2 years ago

テスト、今までのテストが基本的に通ることを確認したいのでOldとして残して、新しい方式でもできることをNewつきのテストで試す方針にしようと思ってます。 この辺も意見募集

forest1040 commented 2 years ago

ParametricGateのgate_factory パラメータIDなしにはParametricGateが使えなくなったのでgate::ParametricXX()の引数が変わっちゃいました。 これが一番でかくて、テストコードもあれこれ書き直さなければならなくなる場所です。

パラメータIDなしの場合、既存のadd_parametric_XXX_gate()についてと同じようにこちらでIDを降ってしまえばよいかもと思いました。

それ以外はOKです！

KowerKoint commented 2 years ago

従来のgate_factoryのほうは従来の「ゲートにパラメータをもたせる」考えが残っているため、まだCircuitと関連づいていないのにintial_angleを指定されたりして結構厳しいです…

KowerKoint commented 2 years ago

gate名前空間の中にinternal名前空間を作ってそこに「次に自動生成するパラメータID」や「initial_angleが与えられてるパラメータとそれに対するinitial_angle」などを持たせる実装も一応可能です。

KowerKoint commented 2 years ago

gate_factoryの旧関数、作りました internal名前空間に区切ったとはいえグローバル変数は増えちゃいました

KowerKoint commented 2 years ago

ParametricQuantumCircuitの旧メソッドで追加されたものには自動的にindex_プレフィックス付きでパラメータIDができて、parametric_gatefactoryの旧関数で追加されたものには自動的に`gateプレフィックス付きでパラメータIDができます。 ユーザーが入力したものはそのままおいてるので衝突がありえなくもないんですが、これにもプレフィックスをつけるとなると構造がややこしくなってしまう(QuantumGate_SingleParameter::get_parameter_id()`で返すのはプレフィックスありなのかなしなのかなど)のでとりあえず放っておきます ちゃんと整備するならParameterKeyを独立したクラスにすべきかとは思います。

kotamanegi commented 2 years ago

get_parameter_count()がゲートの数っぽく捉えられてしまうことがやはりおおそうだったので削除して

遅レスすみませんが、ここは歴史がある部分なので、移行期間としてしばらくの間はexceptionを送出する形にした方が良いかと思います。関数が見つかりませんと言われても解決策を見つけにくいので、関数がなくなったということを告知できるようにしたいです。

KowerKoint commented 2 years ago

等があるのでブランチを作り直して大まかな方針ごとの PR を作ることにします…(最終的に1つだけのPRをマージします)

KowerKoint commented 2 years ago

386 を作りました。

これは、gate factoryを使用したときやParametricGateのコンストラクタを直接読んだときなどのためにサーキット内ではなくparameter名前空間のグローバルにパラメータを保存する方針です。

KowerKoint commented 2 years ago

なお、未実装ですが他のプラントして

  1. パラメータをゲートに持たせることもできるようにして旧来のゲート生成方法ではゲートそのものに持たせるようにする(これも1と同じで2種類の実装が混在)
  2. 互換性を無視して旧来の方法をバッサリ捨てる
KowerKoint commented 2 years ago

387