jubatus/jubatus#448 における変更内容をまとめる。
大雑把には以下の変更を行った。
- 今まではdriverがmixableを持つ場合があった(driverごとにまちまち)
mixable_holder
はdriverが持っているため、各アルゴリズムに対して、mixable_holder
を受け取ってmixableを登録するregister_mixables(mixable_holder&)
という関数を追加した。datum_to_fv_converter
についてもmixableを移動した。
- 今まではアルゴリズムがモデルへの生ポインタを持っている場合があった
- その際、外からは
shared_ptr
にくるんだmixableを渡す - そのために各アルゴリズムは
shared_ptr
経由でmixableを持つ mixable_holder::get_mixables
を使う処理(mixerなど)もこれに追随させた
- アルゴリズムごとにストレージが異なる場合、その違いを透過的に扱えないため
- driverの
get_status
関数においてストレージにアクセスしていたが、アルゴリズムにget_status
関数を追加することで対応した。
anomaly
やrecommender
などは、異なるストレージが混在していたため、モデル型としてstd::string
を用いて、ストレージ側で型の違いを解決していた- MIX処理を簡潔に書けるようにするため、mixableのレベルで型解決するように変更した
- こうすることで、
framework::mixable
が型の変換を勝手にやってくれるようになり、アルゴリズム開発者は冗長なserializationを書かずに済むようになる
- こうすることで、
- この変更のために、次のクラスに
MSGPACK_DEFINE
を追加した。anomaly::lof_entry
(元storage::lof_storage::lof_entry
)common::key_manager
storage::bit_vector
storage::lsh_entry
storage::sparse_matrix_storage
- 新しく追加されたmixableは、ストレージ
*_storage
の定義されているヘッダにmixable_*_storage
という名前で定義した。 - ほとんどのmixableはモデルの型が異なるだけで定義が一緒になったので、
framework::delegating_mixable
に実装をまとめた。- これはアルゴリズムにMIX処理を委譲するだけのmixable。
- これでよいのか、MIX処理自体をアルゴリズムのクラスからmixableに移した方が良いのかについては 要検討 。
- 追加したmixableの一覧
- recommender
mixable_bit_index_storage
mixable_inverted_index_storage
mixable_lsh_index_storage
mixable_recommender_mock_storage
- anomaly
mixable_lof_storage
- recommender
- 削除したmixableの一覧
mixable_recommender
mixable_anomaly_storage
portable_mixer
がstring型以外のモデルに対応していなかったので対応させた。- これらの変更に伴って、主にrecommenderとanomalyにおいてMIX処理における不必要な(de)serializationを削除した
以下、各タスクごとに上記以外の固有の変更について述べる。
classifier_base
のprotectedメンバ変数をprivateに移した- 代わりにprotectedの
get_storage
関数を暫定的に追加した。 get_storage
の戻り値に正しくconstをつけるために、storage_base
の関数get, get2, get3, inp, get_status
をconstメンバ関数に変更した。
- 代わりにprotectedの
- mixableは個別のアルゴリズム実装ではなく
classifier_base
に移動- 同じ
linear_function_mixer
をmixableとして使っているため、暫定的に - 本当はインターフェイスを束ねるベースクラスと、共通のmixableに関する処理をまとめるベースクラスとは分けないといけない
- 特に今後異なるmixableを使うアルゴリズムを追加したくなったら、ここを分けないといけない
- 今は暫定的にここが密結合になっている
- 同じ
特になし
特になし
recommender_storage_base
を削除- ストレージ毎にmixableを分離し、それらを各アルゴリズムが所有するようにしたので、不要となった
recommender_storage.hpp
を削除
anomaly_storage_base
を削除- ストレージ毎にmixableを分離し、それらを各アルゴリズムが所有するようにしたので、不要となった
lof
(recommenderベース)- ストレージが内部に
recommender_base
を持っていたが、アルゴリズム (lof
) が持つように変更- ただしストレージも引き続き
recommender_base
を所有する(処理に用いるため)
- ただしストレージも引き続き
- MIX処理を
lof
とrecommender_base
で完全に分けた lof_storage
の名前空間をstorage
からanomaly
に変更(一貫性を増すため)lof_entry
およびlof_table_t
の定義をlof_storage
のprivateメンバからanomaly
直下に移動- モデルの型として使うため
- ストレージが内部に
light_lof
(nearest_neighborベース)light_lof
のメンバ変数からmixable_nearest_neighbor_
を削除- mixableに関する処理はすべて対応するアルゴリズムが行う(ここでは
nearest_neighbor_engine_
)
- mixableに関する処理はすべて対応するアルゴリズムが行う(ここでは
mixable_weight_manager
をdriver/
からfv_converter/
以下に移動
なるほど。今回の変更では深い意図はなくて、その場しのぎですね。 @suma さんの仰ることには僕も同意です。
このリファクタリングで現状のインターフェイスはあまり壊したくないので、とりあえず仮に置いておくとして、TODOコメントなど足しておけばよいですかね。 その上で
get_status
の役割と設計を議論すると良いかと思います。