search

kazuki / embedded-jubatus-python-obsolete

GNU Lesser General Public License v2.1
2 stars 1 forks source link

リファクタリング #2

Closed kazuki closed 8 years ago

kazuki commented 8 years ago
kmaehashi commented 8 years ago

名前空間をjubatus配下から移動する (Python的に異なるパッケージが同じ名前空間の下にあるのは良くない…？)

jubatus-python-client と embedded-jubatus-python の双方を名前空間パッケージとして宣言しない限り、同名のトップレベルパッケージ名を複数のパッケージで共有することはできません (参考: [1])。 ただし、Python 3.3 以前では、同一ディレクトリに両パッケージをインストールした際の __init__.py の競合などの問題がある([2])ため、ユーザの混乱を避けるためにもトップレベルパッケージ名を別なものに変える方が良いと思います。

[1] http://www.freia.jp/taka/blog/hello-python-3.3b1/ [2] https://www.python.org/dev/peps/pep-0420/#rationale

C-APIのインタフェースを直接公開するのではなく，Pythonでラッピングし，型チェックや型に応じた関数切り替え等を行う

本家 jubatus-python-client に、embedded-jubatus-python と RPC 版 Jubatus を seamless に切り替えらえる機能を9月末頃をメドに追加したいと思っています。理想的には以下のような使い味にしたいです。

from jubatus.classifier.client import Classifier, EmbeddedClassifier

client1 = Classifier("localhost", 9199, "cluster")  # RPCクライアントを生成(既存通り)
client2 = EmbeddedClassifier({"method":"perceptron", "converter":{ ... }})  # embeddedクライアントを生成

# client1 も client2 も train, classify, ... などのメソッドが全く同じIFで使える
# client2に対してdo_mix, get_statusなどembeddedでサポートされないメソッドが呼ばれたら警告(warnings.warn)を出す

ただ、現状RPC I/Fとembedded版(jubatus_core)のI/Fに差分があります。例えば、Classifierのtrainメソッドの引数は:

この差異をどのレイヤで吸収するか検討しているのですが、「Pythonでラッピング」の中に、embedded I/FをRPC I/Fに揃える... というタスクは含まれるでしょうか？

kazuki commented 8 years ago

この差異をどのレイヤで吸収するか検討しているのですが、「Pythonでラッピング」の中に、embedded I/FをRPC I/Fに揃える... というタスクは含まれるでしょうか？

はい．C側でLabeledDatumに対応するのがだるかったので，現状はタプルしか受け付けていませんが Pythonで一度ラップしてしまえばPython側でLabeledDatumをタプルに変換してC側に渡すことが簡単にかけるので，I/Fを完全に揃えるというのもこのタスクに含んでいます

kmaehashi commented 8 years ago

回答ありがとうございます! 9月末のJubatusリリースに入れたいので、本家クライアントにembeddedを結合するためのグルーコードを入れる期間を考慮して、9月頭くらいにはリファクタリングが完了していると大変助かります。もし作業の分担が必要でしたらお知らせください。

kazuki commented 8 years ago

@kmaehashi 遅くなりましたが，リファクタリング(というかCythonで書き直し)にとりかかり， だいたい終わりました．

2点ほど相談があるのですが，

ただ、現状RPC I/Fとembedded版(jubatus_core)のI/Fに差分があります。例えば、Classifierのtrainメソッドの引数は:

  • embedded: [("label", Datumインスタンス)]
  • RPC: [LabeledDatumインスタンス]

とありましたので，LabeledDatumを受け付けるようにしたのですが， Classifierのチュートリアルにあるように， 本家のPythonクライアントはタプルも受け付けているようです． おそらくタプルとLabeledDatumはMsgPackでシリアライズすると 同じデータになるので，問題なく動いているんだと思います．

現状，Embedded版では，ClassifierのみLabeledDatumの代わりにタプルでも受け付けることが出来ますが，他のアルゴリズムではタプルを受け付けることが出来ず，jubatus.*.types で定義された型を利用する必要があります． この点で互換性が保てないケースがあると思いますが，対応不要としても問題ないでしょうか？ (対応が必要であれば，本家側クライアントでの対応 or プルリクお待ちしてます)

それと，モデルのsave/loadは，現在はbytesを返却/bytesを引数に指定するようにしており， RPC版のクライアントと互換性が有りません． もし，ここも戻り値の型的に互換性を保つ必要があるのであれば， {"localhost": 0} を返却し，/tmp にモデルを書き込むことで対応できますが，どうしますか？ (こちらは対応が容易なので，必要であれば対応します)

kmaehashi commented 8 years ago

@kazuki ご対応ありがとうございます。

現状，Embedded版では，ClassifierのみLabeledDatumの代わりにタプルでも受け付けることが出来ますが，他のアルゴリズムではタプルを受け付けることが出来ず，jubatus.*.types で定義された型を利用する必要があります． この点で互換性が保てないケースがあると思いますが，対応不要としても問題ないでしょうか？

タプルを受け付けているのは、過去のバージョンとの互換性を保つため（Jubatus 0.4以前ではLabeledDatumではなくタプルを直接指定していた）なので、この点は対応不要で良いと思います。

それと，モデルのsave/loadは，現在はbytesを返却/bytesを引数に指定するようにしており， RPC版のクライアントと互換性が有りません． もし，ここも戻り値の型的に互換性を保つ必要があるのであれば， {"localhost": 0} を返却し，/tmp にモデルを書き込むことで対応できますが，どうしますか？ (こちらは対応が容易なので，必要であれば対応します)

save, load, save_bytes, load_bytes という4つのメソッドを用意すると良いかなと思いました。

kazuki commented 8 years ago

コメントありがとうございます． タプルを受け付けていたのは過去との互換性ためだったんですね．

save/loadについては対応します．