Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZCA Whitening #28

Merged
merged 8 commits into from
Apr 24, 2018
Merged

ZCA Whitening #28

merged 8 commits into from
Apr 24, 2018

Conversation

wsuzume
Copy link
Contributor

@wsuzume wsuzume commented Apr 23, 2018

Summary

  • Implement ZCA Whitening

Related Issues/Wiki/Resources

ZCA Whitening has strong connection among follow methods.

@tksmd tksmd changed the title Issue/21/zca whitening ZCA Whitening Apr 24, 2018
@tksmd
Copy link
Contributor

tksmd commented Apr 24, 2018

@wsuzume 以下あたりを修正しました。マージしますね。

テストケースについて

  • 一つのテストメソッドで色々とアサーションをしていましたが、それを個別のテストメソッドに分割しました。こちらの方が、テストケースは小さくしておくほうがメンテしやすいかと思います
  • テスト実行で得られる値および、期待する値の変数名を、慣習的に用いられる actualexpected に変更しました。こういった慣習に従っておくと、可読性も高くなりますし、他の人が読むときに読み解きやすくなります
  • transform の結果ですが、調べてみたところ np.linalg.svd の結果がそもそも Intel Mac とその他の環境で異なりうるケースがあることがわかりました。なので、expected の assertion をいくつかのメソッドで外しています。参考 こちらは良いワークアラウンドがあれば、元に戻したいですが、ひとまずはこれですすめます

** WhiteningScaler について**

  • 実装のされていないメソッド ( reset ) の削除
  • _fit 内のメソッドの整理 ( パラメータのチェックをメソッドの最初に移動等、不要な transform 実行の削除 )
  • WhieningScaler のインスタンス属性の整理 ( 必要なもののみに制限 )
  • fit_transform の自前実装をやめて、Mixin の実装に委譲

Notebook について

良い感じと思いました。Notebook が長かったので、最後の図の表示のところの調整だけしました。

@tksmd tksmd merged commit acf7105 into development Apr 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants