#129_欠損対応(重み付き)TSOM3の追加

[forusufia]

Description 説明 重み付き版のTSOMアルゴリズムで tsom3_weighted.py を追加しました

close #129

Type of change 変更の種類

• [x] New feature (non-breaking change which adds functionality)

How Has This Been Tested? どのようにテストしたか？ somf\tests\tsom\tsom3_weighted\tsom3_weighted.py とのペアプロをお願い致します。

• [x] 数式から重み付きのTSOM3 をプログラム化
• [ ] somf\tests\tsom\tsom3_weighted\tsom3_weighted.py と allclose

ペアプロ用に somf\tests\tsom\tsom3_weighted\allclose_tsom3_weighted_harada_vs_another.py もありますので、適宜モデルの呼び出し部分をご自分のプログラムに書き換えて活用ください。

[forusufia]

数式再掲

[ae14watanabe]

これ、somfの既存のクラスに機能追加という形で実現はできないでしょうか？

[ae14watanabe]

メンション飛ばし忘れた @forusufia いやね、ちょっとあまりにもtsom関係のクラスが多すぎるなと思って（ #128 で問題提起はしてる）

[forusufia]

@ae14watanabe クラス呼び出しの時の引数を一個増やしてモード切替するような感じでしょうか？ weight = "enable" or "disable" みたいな

[ae14watanabe]

いや、重み（欠損の場合は2値が入ってるマスク？）を渡したらそれ使うし、渡さなかったら使わないみたいな仕様でいいんじゃないでしょうか？TSOM2 https://github.com/furukawa-laboratory/somf/pull/101 の仕様と合わせると良さそう。

[ae14watanabe]

tsom3_weighted.py って tsom3.py を自分でコピーしてそれを修正してる感じですか？もしそうなら差分をtsom3.pyに反映させてプルリク出すと良さそう。 完全に1からフルスクラッチしてるなら渡辺の提案は新しい作業することになるのでちょっと面倒そうですね。

[takuro-Ishida]

おそらくtsom3.pyをコピーしてやってる感じだと思う.

とりあえず，僕は原田くんがいう通り実装してallcloseで一致するか確かめます

[ae14watanabe]

マージするなら機能追加って感じだと思ってて、それは追い追いやって欲しいけど、めっちゃ急ぎってわけでもないので余裕のある時に @forusufia にはやってもらえばいいかなと思う。 @takuro-Ishida がペアプロ用に先行してクラスを上げるのは良さそうですね。 @forusufia 自身の研究の作業にも必要だと思うので。

[forusufia]

わかりました。余裕あるときに機能追加の形に直します！

[takuro-Ishida]

@forusufia 石田も実装してtestしてみました. allcloseはTrueになってません

[takuro-Ishida]

原因の一つとしてtsom3_weighted.pyのYの更新式が間違っていることが挙げられます. 具体的には，YをU1を使って書いてると思うのですがその式が数式から間違ってると思います(以前僕と話した内容を元に実装してると思うのですが，それが間違いですね)

詳細は以下のコメントで

[takuro-Ishida]

[takuro-Ishida]

[takuro-Ishida]

なので，Yの更新部分を修正してもう一度お願いします！ 字が汚いのは愛嬌ってことで笑

[forusufia]

シンプルに式変形をミスしていたようです・・・直します！

[forusufia]

あ・・・この変形もマズイような

[takuro-Ishida]

自分もまったく同じ式過程で同じ結果になりました.

一応，変形前とあとでプログラム上でallcloseしてTrueになったので間違ってはないと思っています

[takuro-Ishida]

気になっていることがあればぜひお聞きしたいです！

[forusufia]

g を右辺、左辺間で移動してしまう理解はまずいかなと思いまして

石田さんがやっているように、分数の形で式の途中に”発生？”させてあげて、スッと置き換えるのがいいのかなと

今回の場合最終形は同じなのですが、ちょっと気になったという話です

[takuro-Ishida]

①の変形は問題ないと思う 問題なのは，一番最後の式の分母の ~g^(4)~ g^(1)は動かせないところだと思います

だから原田くんの式変形も問題なさそうだけどね〜

[forusufia]

g^(4) が動かせないのといっしょで、①を作る時にg^(1)を左辺に移動するのもまずいかなーというところです

[forusufia]

2020/04/06 メモ ↑上記の件、お話で解決しました

[forusufia]

@takuro-Ishida tsom3_weighted.py (harada) self.Y = np.sum(H1[:, :, None, None, None] (G1[:, :, :, None] self.U1)[:, None, :, :, :], axis=0) / np.sum(H1[:, :, None, None] * G1[:, None, :, :], axis=0)[:, :, :, None]

K1K2K3*D

tsom3_weighted_ishida.py self.Y=np.sum(H1[:,np.newaxis,np.newaxis,:,np.newaxis] (G1[:,:,:,np.newaxis]U1[np.newaxis,:,:,:,:] , axis=3) / np.sum(H1[:,np.newaxis,np.newaxis,:,np.newaxis] * G1[np.newaxis,:,:,:,np.newaxis] ,axis=3)

allclose が一致しなかったのでプログラムを見渡していました H1 やG1 の次元数がだいぶ違うみたいなのですが、原因はこのあたりなのでしょうか・・・

[takuro-Ishida]

broadcastのやり方が２人で違うので(石田は一気にやってる)そこが問題かどうかはわからないですね.

細かくみていくしかないかと思います.(分母のみであってるか、分子のみであってるかなど)

[takuro-Ishida]

あと @noguchikazuki にレビューをしてもらう時は直接声をかけたほうがいいかなと思います！ githubを日常的に活用しているわけではないので

[forusufia]

@takuro-Ishida ひとまず一つ差異がありました

@forusufia のtsom3 H1 = np.exp(-distance1 / (2 pow(sigma1, 2)))

@takuro-Ishida のtsom3 H1=np.exp(-0.5Dist1/(2 pow(sigma1, 2)))

H2, H3 も同様で、Dist の前に0.5 が掛かっているかいないかが異なります この0.5って分母の２で割るのが２重になっていませんか？

[takuro-Ishida]

H2, H3 も同様で、Dist の前に0.5 が掛かっているかいないかが異なります この0.5って分母の２で割るのが２重になっていませんか？

なるほど. そこは自分が間違ってますね.

ありがとうございます

[forusufia]

エポック５回目でk3 の値が違ってきて エポック６回目からY などに影響が出始めてるのがわかるのですが

@takuro-Ishida @noguchikazuki 延々デバッグしてますが手詰まり感があるので、ちょっと別の人の視点が欲しいです お助けください...

[forusufia]

@takuro-Ishida めちゃくちゃシンプルに見落としてました tsom3_weighted_ishida.py の勝者決定のところでG が掛かってなかったみたいです・・・

修正したら一応一致してるっぽいです

チェックお願いします～

[takuro-Ishida]

@forusufia 了解です！ 自分も検証して確かめてみます！

[takuro-Ishida]

結果的に俺がひたすら間違ってたって感じか...

申し訳ないです

[forusufia]

いえいえ～　 途中の計算など色々と見直ししたので 助かっています

[ae14watanabe]

@forusufia お疲れ様です！ 確認なんやけど、***.npyみたいなファイルってデバッグ用よね？ gitは一旦commitしちゃうと、ファイルを削除しても「ファイルを削除した」というコミットが残るだけで内部ではファイルを保存してるので、こういうファイルはコミットしない方が吉な気がします。リポジトリの容量が増えていってしまうので…

[takuro-Ishida]

@ae14watanabe リポジトリの容量って有限？

[forusufia]

@ae14watanabe デバッグ用です！ 間違えて追加しちゃってたみたいですね・・・気を付けます

[ae14watanabe]

@forusufia できれば消せませんか？ちょっとめんどくさそうだけど

[forusufia]

@ae14watanabe ファイルは消しましたが、コミット自体をですか？

[forusufia]

プログラムをいったん退避して戻ればいいんでしょうか

[ae14watanabe]

@forusufia うん、ファイルを追加したという事実を消さないと、表面的にファイルを削除しても.gitの中に残ってるんですよファイルが。それがリポジトリの容量を増やしてしまうので。

[ae14watanabe]

@takuro-Ishida リポジトリの容量自体は有限じゃないけど、クローンする人がだるいよねってだけ。まぁ別にいいかこれぐらい。

[ae14watanabe]

やっぱめんどくさいね。やめとこう。

[ae14watanabe]

あと別件ですけど、reviewでapproveにするのはTSOMへの機能追加という形で実装できた時まで待った方がいいと思います。これでマージしてもtestにしか反映されないので…

[takuro-Ishida]

reviewでapproveにするのはTSOMへの機能追加という形で実装できた時まで待った方がいいと思います。

確かに. model/tsom3/tsom3.pyに統合するのが一番ですが，それが大変なのであれば model/tsom3の中にtsom3_weighted.pyを移植するでもいいと個人的には思います.

[ae14watanabe]

@takuro-Ishida @forusufia 今回の重み付きって普通のTSOMの一般化（普通のTSOMは重みが全て1という特殊な状況）って認識で合ってますか？そうじゃなくてアルゴリズムがかなり違うなら別のクラスとして用意するのもアリかなと思って。わざわざ別のクラスとして用意してまで焦ってマージする必要もないと個人的には思ってます。

[takuro-Ishida]

今回の重み付きって普通のTSOMの一般化（普通のTSOMは重みが全て1という特殊な状況）って認識で合ってますか？

あってます.当面は欠損データを扱う予定です.

わざわざ別のクラスとして用意してまで焦ってマージする必要もないと個人的には思ってます。

そこは，Assignees(@forusufia )次第かなと. 緊急で使う必要があるのであれば，ひとまず別クラスで入れてしまうのもいいのかなとは思ってます.(リポジトリの容量は圧迫してしまいますが...) おっしゃる通りtsom3.pyに機能追加という形で入れるのが一番ですが.