chainerにコミットして学んだこと、学ばなきゃいけないと感じたこと
先日大型OSSへのコミットという自分の中での一つの目標を達成することができました。
といってもまだまだ変更は小規模ですが、小さなコミットでも考えさせられる事があって収穫は大きく楽しいので習慣にしたいと思ってます。
コードレビューも丁寧にやっていただけるし。
でも、レビューで指摘を受けずに一発マージを目指したいんよ~。
以下にOSSにスムーズにコミットするうえで押さえるべきポイント(と私が勝手に思っているもの)をまとめておきます。
- コントリビューションしたいOSSを決める
- コントリビューションガイドを読む
- コミットする内容を決める
- コミットすることを宣言する
- githubの使い方をよく理解しておく
- プロジェクトのコード(テスト、docstringも含めて)をよく読む
- 既にクローズ済のPRで、議論が活発にされているものをちょっと読んでみる
- 教科書を読む
- 最後にもう一度チェック、自戒をこめて
コントリビューションしたいOSSを決める
自分が使っているOSSがいいです。
どんなに小さいコントリビューションでもそれなりにコードリーディングなどに時間と体力を使う必要があり、興味がないプロジェクトにはモチベーションを維持しづらいです。
コントリビューションガイドを読む
それなりの規模のOSSでは整っているところが多いです。コーディング規約、テストの考え方、その他お作法が書いてありますので二、三週読みましょう。
コミットする内容を決める
自分で使っているうちにバグに気づいたり、追加したい機能があればそれを足せばいいですが、
人気があるライブラリは素人がちょっと触ったくらいじゃバグや追加したい機能はなかなか見つかりません。。
そんなときは既に開いているissueを見てみます。
といってもissueの中には大規模な変更が必要なものや、抽象度が高くライブラリに対する深い理解が必要なもの、難解な論文の実装など難易度の高いものがあったり、そもそもユーザーの認識間違いで開かれたissueで実装変更が必要ないものもあります。
取り組みやすいissueを見つけるためにはタグが有効だったりします。
chainerで言えば"contribution-welcome"がついているタグを見てみましょう。
scikit-learnで言えばEasyですね。
中にはテストを除くと数行規模の実装でできるようなのなどとっつきやすいissueが並んでいて、とっかかりにはお勧めです。
コミットすることを宣言する
chainerを含め、WIP(work in progress)でプルリクエストを投げる事を許可しているプロジェクトがあります。
折角がんばってプルリクエストを完成させても、他の人に先を越されて無駄になっちゃうと悲しいので、
ある程度自分でやれる、そしてやると決めたissueにはWIPで投げて「私がやりまっせ!」とツバつけておきましょう。
PRの代わりにissueで私がやるよーといってるOSSもありますね。
プルリクエスト作成画面で本文に、#(issue番号)と打つと、issueで参照されるので、この人がやろうとしてるんだなーとかもう誰かが実装したんだなーみたいなのがわかります。
参照されてるissueの例↓
そして当然、他の人がツバつけてるissueは避けておきましょう。
githubの使い方をよく理解しておく
例えばchainerはgithub flowに従っているので、必要な操作を理解しておきます。
マスターブランチで作業せずトピックブランチを作るなど。具体的な操作全貌は↓が詳しいです。
GitHubへpull requestする際のベストプラクティス - hnwの日記
プロジェクトのコード(テスト、docstringも含めて)をよく読む
実装もissueの要件を満たしてる、テストもパスしてる、これでいけるやん!と思っちゃうのですが、まだマージへの壁は残っています。
というのも、コントリビューションガイドにはなっていないけれども、chainerならchainer的な書き方、流儀、文化とよべるものが存在し、それに合わないコードを受け入れないことによって、統一された読みやすいライブラリを保っているのです。
最初から絶対に完璧であるべきとは思いません(し、私もできてません)が、やれる努力はしましょう。
例えば新しいディープラーニングの損失関数を足したい場合は、既存の損失関数の実装というように、よほど斬新な機能追加を検討していない限り、似たような機能およびそれらのドキュメント、テストが存在するのでそれを読み込んで参考にします。
まず目指すべきなのは"最高にイノベーティブなコード"ではなく"そこに初めからあったようなコード"です。
そのために自分が記述したコードの何倍ものコードのリーディングをしましょう。テストもね。その過程が収穫になります。
既にクローズ済のPRで、議論が活発にされているものをちょっと読んでみる
どういった指摘があるのかわかるのでお勧めです。
chainerで流し読みをしていると、rangeではなくsix.moves.rangeを使ってくれ、テストを作ってくれ、gpu版のテストも作ってくれ、withを使わない、複数の変更意図を含んでいるのでブランチを分けてくれといったものが多いです。
中の人はレビュー大変そう…。
教科書を読む
機械学習周り固有の事情かもしれませんが、ライブラリをいじるのに数学的な基礎知識が必要なケースが多々あります。
ディープラーニングライブラリだとバックプロパゲーションってなに?みたいな状態だと、多くの機能実装は厳しかったりします。
例えば上のcontribution-welcomeタグのスクショに写っている5つのissueのうち、三つはバックプロパゲーションの実装を含んでいたり。
最後にもう一度チェック、自戒をこめて
- issueを完全に理解しているか?
- flake8でチェックしたか?
- ローカルでテストはパスしたか?
- 自分でもう一度差分を落ち着いて読んだか?
なお、多くのオープンソースコミュニティでは、提出したのが完全でないプルリクエストだったとしても、ベストを尽くしていて、謙虚に議論していて、コミュニティに貢献したいと思っているコミッタは常に歓迎している…(と、信じてます)。
chainerは非常にcheerfulな雰囲気で、しょぼしょぼの規模のPRでもLGTM!といってくれます。
最初見たときはLong-Short Term Memoryの亜種かと思いましたが、どうやらLooks Good To Me!と褒めてるようですよ!