driverのリファクタリング内容

jubatus-refactoring-drivers.rst

driverのリファクタリング内容

jubatus/jubatus#448 における変更内容をまとめる。

概要

大雑把には以下の変更を行った。

mixableのアルゴリズムへの移動

• 今まではdriverがmixableを持つ場合があった（driverごとにまちまち）
• mixable_holder はdriverが持っているため、各アルゴリズムに対して、 mixable_holder を受け取ってmixableを登録する register_mixables(mixable_holder&) という関数を追加した。
• datum_to_fv_converter についてもmixableを移動した。

モデルへのアクセスはすべてmixable経由で行うように

• 今まではアルゴリズムがモデルへの生ポインタを持っている場合があった

mixable_holder がmixableを所有するように

• その際、外からは shared_ptr にくるんだmixableを渡す
• そのために各アルゴリズムは shared_ptr 経由でmixableを持つ
• mixable_holder::get_mixables を使う処理（mixerなど）もこれに追随させた

アルゴリズムの外からストレージにアクセスするコードを削除

• アルゴリズムごとにストレージが異なる場合、その違いを透過的に扱えないため
• driverの get_status 関数においてストレージにアクセスしていたが、アルゴリズムに get_status 関数を追加することで対応した。

ストレージ毎に異なるモデル型とmixableを定義

• 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
• 削除したmixableの一覧
  • mixable_recommender
  • mixable_anomaly_storage
• portable_mixer がstring型以外のモデルに対応していなかったので対応させた。
• これらの変更に伴って、主にrecommenderとanomalyにおいてMIX処理における不必要な(de)serializationを削除した

以下、各タスクごとに上記以外の固有の変更について述べる。

classifier

• classifier_base のprotectedメンバ変数をprivateに移した
  • 代わりにprotectedの get_storage 関数を暫定的に追加した。
  • get_storage の戻り値に正しくconstをつけるために、 storage_base の関数 get, get2, get3, inp, get_status をconstメンバ関数に変更した。
• mixableは個別のアルゴリズム実装ではなく classifier_base に移動
  • 同じ linear_function_mixer をmixableとして使っているため、暫定的に
  • 本当はインターフェイスを束ねるベースクラスと、共通のmixableに関する処理をまとめるベースクラスとは分けないといけない
    • 特に今後異なるmixableを使うアルゴリズムを追加したくなったら、ここを分けないといけない
    • 今は暫定的にここが密結合になっている

regression

特になし

nearest_neighbor

特になし

recommender

• recommender_storage_base を削除
  • ストレージ毎にmixableを分離し、それらを各アルゴリズムが所有するようにしたので、不要となった
• recommender_storage.hpp を削除

anomaly

• 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_ ）

fv_converter

• mixable_weight_manager を driver/ から fv_converter/ 以下に移動

get_status の役割・要求があまり決まっていないので、アルゴリズムにget_statusを置くのは反対です。仮という形で置いておく分には良いと思います。
mixable（とストレージの区別もしつつ、では） get_status で何を返すべきか、どんな情報がアルゴリズムごとに異なるのか決まっていない・議論されていないため、共通化する部分・共通化できない部分はどこなのか整理する必要がありそうです。
また、mixable依存の任意の情報の取得には、map<string, string> によるget_status よりもQt(QObject)のpropertyといった仕組みが向いていると思います。

なるほど。今回の変更では深い意図はなくて、その場しのぎですね。 @suma さんの仰ることには僕も同意です。
このリファクタリングで現状のインターフェイスはあまり壊したくないので、とりあえず仮に置いておくとして、TODOコメントなど足しておけばよいですかね。 その上で get_status の役割と設計を議論すると良いかと思います。