OpenCVのバグ(Pull Request)
の編集
http://tessy.org/wiki/index.php?OpenCV%A4%CE%A5%D0%A5%B0%28Pull+Request%29
[
トップ
] [
編集
|
差分
|
履歴
|
添付
|
リロード
] [
新規
|
一覧
|
検索
|
最終更新
|
ヘルプ
]
-- 雛形とするページ --
(no template pages)
[[OpenCVのバグ(trac)]] この度、OpenCVへのPull Request がマージされたので、その時の記録とかを書いておきます。 * Itroduction [#p200d0e4] -First, always check the https://github.com/opencv/opencv/wiki/How_to_contribute * 時系列 [#q5b6e51d] ** testのfailに気づく [#h4d00b0d] -ことの始まりは[[ODROID-C2]]を買って、その上でOpenCVのtestを走らせたことから -何故か、testの一部がFAILEDとなる -見てみると、hal_intrinというテストが失敗していた -具体的には、HW命令を使用して平方根(square root)を求めた結果と、std::sqrtの結果が一致しないというエラーだった /home/odroid/opencv-fork/modules/core/test/test_intrin.cpp:578: Failure Value of: resF[i] Actual: 11.054863929748535 Expected: std::sqrt(data1[i]*data1[i] + data2[i]*data2[i]) Which is: 11.054862976074219 -ODROID-C2は、純粋に64bitArmであり¬e{raspberry-pi-3:話題を呼んだラズパイ3は、OSは32bit。ODROID-C2は64bitのUbuntu16.04が動くのが特徴};、このテストをfixしてみようと気づいた ** エラーしている箇所を眺めてみる [#qf81f66d] -内容は、11.054862976074219になるべき結果が、 -なぜか、11.05486''3929748535''になっているというエラー -もともとも、HW命令を使った計算では、[[ニュートン法>メモ#f9155386]]で&mimetex(\frac{1}{\sqrt{x}});を求めて、そこから&mimetex(\sqrt{x});を求めるアプローチ -反復することで、精度があがるので、まぁ、もう1回ぐらい計算を繰り返したら微妙な誤差も消えるだろう、と思って、試しに、もう1回繰り返してみた。 #geshi(c++){{ float32x4_t e = vrsqrteq_f32(x1); e = vmulq_f32(vrsqrtsq_f32(vmulq_f32(x1, e), e), e); e = vmulq_f32(vrsqrtsq_f32(vmulq_f32(x1, e), e), e); + e = vmulq_f32(vrsqrtsq_f32(vmulq_f32(x1, e), e), e); return v_float32x4(vmulq_f32(x.val, e)); } }} -こんな感じに、eに入る値がどんどん&mimetex(\frac{1}{\sqrt{x}});に近づいていくという寸法。 -ちなみに、ArmのNEON命令を使っているので、float4つ分が一気に計算される。 -ベクタの要素のうち1つが、計算結果がズレ過ぎている、としてFAILEDになっていた ** 修正案を実装してPR(Pull Request)する [#n6a10b6c] -試しに実装したらテストのFAILEDがPASSした -なので、1行だけの修正だし、これで良かれと思って、PRを投げることにした *** How to contributeページを読む [#q4e09cbe] -ここからが大事なところ。OpenCVでは、[[How to contirbute>https://github.com/opencv/opencv/wiki/How_to_contribute]]¬e{how-to-contribute-opencv:[[How to contirbute>https://github.com/opencv/opencv/wiki/How_to_contribute]], 2022-05-20 18:53版, 2022-07-07閲覧};というページがある -ここでは、PRを投げる前に読むべき大事な点がたくさん書かれているので、本当に読んだ方が良い -Gitのお作法だけでなく、コーディングスタイルに関しても書かれている。 -そして、なるだけ、機械的なチェックをパスするように指導されている。その一部をここで紹介 -''注意'' ここで書いている情報は2016-05-16時点の情報なので、これを読んでPRを投げる人は、必ずWikiで最新版の情報を参照して下さい *** Githubのお作法 [#h46e1c5b] -この節だけはOpenCV以外のプロジェクトにも共通する、gitのfork-mergeに関するお話¬e{github-creating-pull-request:[[Creating a pull request - User Documentation>https://help.github.com/articles/creating-a-pull-request/]], 2016-05-16閲覧};¬e{github-fork-a-repo:[[Fork A Repo>https://help.github.com/articles/fork-a-repo/]], 2016-05-16閲覧}; -手順としては #ref(000-fork-and-pull-request.png) +opencv/opencv リポジトリを自身のリポジトリに fork する +自身のリポジトリから、ローカルに clone する +ローカルでfixして、commitする +自身のリポジトリにpushする +GithubのインタフェースからPull Request を作る -という感じです。詳細は[[githubの説明(リポジトリをforkする)>https://help.github.com/articles/fork-a-repo/]][[githubの説明(Pull Requestを使う)>https://help.github.com/articles/using-pull-requests/]]当たりを参照して下さい *** 使用するブランチ [#q2b90933] -以降の節はOpenCVに限った話 -OpenCVは2016年5月16日現在、3.0系列と2.4系列の2本の系列があります。 --OpenCV2.4系列は既に「新機能の追加は行わない」とアナウンスされており、bug fixだけを取り込むこととなっています --OpenCV3.0系列がメインの開発対象となっており、新規機能はこちらに取り込むこととなっています -なので、bug fix ならば2.4系列、もしくは両方に、新規機能の実装ならば3.0系列に、それぞれPRを投げることになります。 -今回私が投げたPRはbug fixなので、本来は2.4系列にも投げるべきなのですが、そもそもhal_intrinがHALモジュール(Hardware Acceleration Layer)という、3.0から入った機能を使った話なので、今回はmaster(3.0系列)にPRを投げることにしました。 *** pre-commit hook [#c409f59e] -前述の手順でローカルにリポジトリをcloneした後、作業をする前に、pre-commit hook を用意すると、みんな幸せになれます -具体的には、cloneしたリポジトリの .git/hooks/pre-commit.sample というファイルを、 .git/hooks/pre-commit にリネームします。 -この.gitディレクトリは通常隠しディレクトリなので、うまいこと、やってください -これをすることで、White spce に関する規約をcommit 直前にgitがチェックしてくれます。こうすることで、自然とすべてのPRがWhite spaceの規約に則ることになります。 --ちなみに、OpenCVはインデントはスペース4つです。タブは使いません。 -例えばインデントにタブを使うと、Commit自体がエラーになります **PRを書くコツ [#h311d670] -最近、PRを書く際に、テンプレートが表示されるようになりました。 -テンプレートには、大きく、**What does this PR change **という節を書くようになっています -ここに、分かりやすく --このPRで何をしたいのか --このPRを何故書いたのか --などを分かりやすく説明します -私のオススメとしては、この説明文を上手く書けないようなPRは、恐らく細切れなPRに分割した方が良いと思います **BuildBot [#eabbfe16] -PRを投げたあとの全体的な流れは[[この図>http://code.opencv.org/projects/opencv/wiki/How_to_contribute/#Here-is-the-flow-chart-of-the-process]]が参考になると思います。 -PRを投げたあとは、機械テストが実行されます。 -2016年5月16日時点では --Linux x64ビルド --Win7 x64ビルド(VS2013) --Win10 x64ビルド(VS2015) --Mac --Android armeabi-v7a --OpenCL --Linux x64 Debug --Docs --iOS --の9種類のビルド+テストが実行されます -オプショナルに --OpenCL Intel --Win32 --Arm --Android pack --などが実行されますが、実行される条件は不明です。 -この機械テストは、自分のローカルで再現可能です。そのことに関してはまたその内書きます -全ての自動ビルドに、Warningもなく、エラーもなく、その上で全てのテストがpassすれば、晴れてマージ可能となります。 -ただし、マージされるかどうかは、Reviewが入ります **Review [#cd975f57] -Reviewのためには、Intelのメンバが承認することが必要です。 -マージする前に直すべき点、コードの書き方、テストが抜けている、など様々な指摘が来ますので、真摯に直しましょう。 -ReviewでOKが出ると、Intelのメンバがアサインされます -アサインされたメンバが絵文字のサムアップ¬e{thumb-up:どうもこのサムアップの絵文字はgithub公式のものらしくて、':+1'と書くと自動的に絵文字に変換されるらしい};を書くと、自動的にリポジトリにマージされます -当然英語でやり取りをする必要があります。頑張りましょう。 ***Reviewでの指摘 [#c7293d64] -私の修正は、自分自身でも、イマイチピンと来なかったのですが、指摘で --ニュートン法を繰り返すのはオーバースペック --11.054863929748535と11.054862976074219はhex表現で0x4130E0B9と0x4130E0B''9''で、そもそも1ULPSしか違わない --そんなbit exactに計算結果を比較しているのがナンセンス --そして、よく見れば等しいかどうかの比較において、doubleとして比較されていたのが原因 --という指摘がありました。 -私自身、PRを投げた経験と同じぐらい、この指摘で多くのことを学びました。 -つまり問題の根源は計算でなく、floatの比較にEXPECT''_DOUBLE_''EQというマクロを使ってることでした。 -なので、そこを適切に直して、私のPRは無事マージされました -ちなみにEXPECT_FLOAT_EQ内では、hex表現のfloatが、いくつ離れているか(ULPS)という点でtrueかfalseを返します。 -ちなみに、このhex表現のfloatがいくつ離れているか、というのは[[ULP>https://en.wikipedia.org/wiki/Unit_in_the_last_place]]という単位で表されます。 -丸め誤差を考慮し、最大でULP4つ分まで離れていても同じ浮動小数点である、と判定する仕組みになっています。 **マージ(Squash) [#a8f9d6d2] -Reviewや、BuildBotでの修正の過程で複数のcommitがpushされることがあります -通常、その過程を全て1つのcommitにまとめる(Squashする)ことが要求されます --branch元にgit reset --mixedする --pushした先のbranchをmasterにmergeする。その際、--squashオプションを付ける --再度commitする --再度pushする。この際、SHA1値が変わってるので、push --forceする * 感想 [#xaf58c3d] -テスト、すげー。そして大事 -一発でテストに通ると思わないほうが良い。テストでfailしたら恥ずかしいと思うけれど、恥じずに直す -何が問題なのか、ちゃんと説明できないなら、良く精査した方が良い *過去に投げたOpenCVのPull Request [#v68f1c35] [[Pull Requests · opencv/opencv>https://github.com/opencv/opencv/pulls?utf8=%E2%9C%93&q=is%3Apr+author%3Atomoaki0705]] [[:OpenCV]]
タイムスタンプを変更しない
[[OpenCVのバグ(trac)]] この度、OpenCVへのPull Request がマージされたので、その時の記録とかを書いておきます。 * Itroduction [#p200d0e4] -First, always check the https://github.com/opencv/opencv/wiki/How_to_contribute * 時系列 [#q5b6e51d] ** testのfailに気づく [#h4d00b0d] -ことの始まりは[[ODROID-C2]]を買って、その上でOpenCVのtestを走らせたことから -何故か、testの一部がFAILEDとなる -見てみると、hal_intrinというテストが失敗していた -具体的には、HW命令を使用して平方根(square root)を求めた結果と、std::sqrtの結果が一致しないというエラーだった /home/odroid/opencv-fork/modules/core/test/test_intrin.cpp:578: Failure Value of: resF[i] Actual: 11.054863929748535 Expected: std::sqrt(data1[i]*data1[i] + data2[i]*data2[i]) Which is: 11.054862976074219 -ODROID-C2は、純粋に64bitArmであり¬e{raspberry-pi-3:話題を呼んだラズパイ3は、OSは32bit。ODROID-C2は64bitのUbuntu16.04が動くのが特徴};、このテストをfixしてみようと気づいた ** エラーしている箇所を眺めてみる [#qf81f66d] -内容は、11.054862976074219になるべき結果が、 -なぜか、11.05486''3929748535''になっているというエラー -もともとも、HW命令を使った計算では、[[ニュートン法>メモ#f9155386]]で&mimetex(\frac{1}{\sqrt{x}});を求めて、そこから&mimetex(\sqrt{x});を求めるアプローチ -反復することで、精度があがるので、まぁ、もう1回ぐらい計算を繰り返したら微妙な誤差も消えるだろう、と思って、試しに、もう1回繰り返してみた。 #geshi(c++){{ float32x4_t e = vrsqrteq_f32(x1); e = vmulq_f32(vrsqrtsq_f32(vmulq_f32(x1, e), e), e); e = vmulq_f32(vrsqrtsq_f32(vmulq_f32(x1, e), e), e); + e = vmulq_f32(vrsqrtsq_f32(vmulq_f32(x1, e), e), e); return v_float32x4(vmulq_f32(x.val, e)); } }} -こんな感じに、eに入る値がどんどん&mimetex(\frac{1}{\sqrt{x}});に近づいていくという寸法。 -ちなみに、ArmのNEON命令を使っているので、float4つ分が一気に計算される。 -ベクタの要素のうち1つが、計算結果がズレ過ぎている、としてFAILEDになっていた ** 修正案を実装してPR(Pull Request)する [#n6a10b6c] -試しに実装したらテストのFAILEDがPASSした -なので、1行だけの修正だし、これで良かれと思って、PRを投げることにした *** How to contributeページを読む [#q4e09cbe] -ここからが大事なところ。OpenCVでは、[[How to contirbute>https://github.com/opencv/opencv/wiki/How_to_contribute]]¬e{how-to-contribute-opencv:[[How to contirbute>https://github.com/opencv/opencv/wiki/How_to_contribute]], 2022-05-20 18:53版, 2022-07-07閲覧};というページがある -ここでは、PRを投げる前に読むべき大事な点がたくさん書かれているので、本当に読んだ方が良い -Gitのお作法だけでなく、コーディングスタイルに関しても書かれている。 -そして、なるだけ、機械的なチェックをパスするように指導されている。その一部をここで紹介 -''注意'' ここで書いている情報は2016-05-16時点の情報なので、これを読んでPRを投げる人は、必ずWikiで最新版の情報を参照して下さい *** Githubのお作法 [#h46e1c5b] -この節だけはOpenCV以外のプロジェクトにも共通する、gitのfork-mergeに関するお話¬e{github-creating-pull-request:[[Creating a pull request - User Documentation>https://help.github.com/articles/creating-a-pull-request/]], 2016-05-16閲覧};¬e{github-fork-a-repo:[[Fork A Repo>https://help.github.com/articles/fork-a-repo/]], 2016-05-16閲覧}; -手順としては #ref(000-fork-and-pull-request.png) +opencv/opencv リポジトリを自身のリポジトリに fork する +自身のリポジトリから、ローカルに clone する +ローカルでfixして、commitする +自身のリポジトリにpushする +GithubのインタフェースからPull Request を作る -という感じです。詳細は[[githubの説明(リポジトリをforkする)>https://help.github.com/articles/fork-a-repo/]][[githubの説明(Pull Requestを使う)>https://help.github.com/articles/using-pull-requests/]]当たりを参照して下さい *** 使用するブランチ [#q2b90933] -以降の節はOpenCVに限った話 -OpenCVは2016年5月16日現在、3.0系列と2.4系列の2本の系列があります。 --OpenCV2.4系列は既に「新機能の追加は行わない」とアナウンスされており、bug fixだけを取り込むこととなっています --OpenCV3.0系列がメインの開発対象となっており、新規機能はこちらに取り込むこととなっています -なので、bug fix ならば2.4系列、もしくは両方に、新規機能の実装ならば3.0系列に、それぞれPRを投げることになります。 -今回私が投げたPRはbug fixなので、本来は2.4系列にも投げるべきなのですが、そもそもhal_intrinがHALモジュール(Hardware Acceleration Layer)という、3.0から入った機能を使った話なので、今回はmaster(3.0系列)にPRを投げることにしました。 *** pre-commit hook [#c409f59e] -前述の手順でローカルにリポジトリをcloneした後、作業をする前に、pre-commit hook を用意すると、みんな幸せになれます -具体的には、cloneしたリポジトリの .git/hooks/pre-commit.sample というファイルを、 .git/hooks/pre-commit にリネームします。 -この.gitディレクトリは通常隠しディレクトリなので、うまいこと、やってください -これをすることで、White spce に関する規約をcommit 直前にgitがチェックしてくれます。こうすることで、自然とすべてのPRがWhite spaceの規約に則ることになります。 --ちなみに、OpenCVはインデントはスペース4つです。タブは使いません。 -例えばインデントにタブを使うと、Commit自体がエラーになります **PRを書くコツ [#h311d670] -最近、PRを書く際に、テンプレートが表示されるようになりました。 -テンプレートには、大きく、**What does this PR change **という節を書くようになっています -ここに、分かりやすく --このPRで何をしたいのか --このPRを何故書いたのか --などを分かりやすく説明します -私のオススメとしては、この説明文を上手く書けないようなPRは、恐らく細切れなPRに分割した方が良いと思います **BuildBot [#eabbfe16] -PRを投げたあとの全体的な流れは[[この図>http://code.opencv.org/projects/opencv/wiki/How_to_contribute/#Here-is-the-flow-chart-of-the-process]]が参考になると思います。 -PRを投げたあとは、機械テストが実行されます。 -2016年5月16日時点では --Linux x64ビルド --Win7 x64ビルド(VS2013) --Win10 x64ビルド(VS2015) --Mac --Android armeabi-v7a --OpenCL --Linux x64 Debug --Docs --iOS --の9種類のビルド+テストが実行されます -オプショナルに --OpenCL Intel --Win32 --Arm --Android pack --などが実行されますが、実行される条件は不明です。 -この機械テストは、自分のローカルで再現可能です。そのことに関してはまたその内書きます -全ての自動ビルドに、Warningもなく、エラーもなく、その上で全てのテストがpassすれば、晴れてマージ可能となります。 -ただし、マージされるかどうかは、Reviewが入ります **Review [#cd975f57] -Reviewのためには、Intelのメンバが承認することが必要です。 -マージする前に直すべき点、コードの書き方、テストが抜けている、など様々な指摘が来ますので、真摯に直しましょう。 -ReviewでOKが出ると、Intelのメンバがアサインされます -アサインされたメンバが絵文字のサムアップ¬e{thumb-up:どうもこのサムアップの絵文字はgithub公式のものらしくて、':+1'と書くと自動的に絵文字に変換されるらしい};を書くと、自動的にリポジトリにマージされます -当然英語でやり取りをする必要があります。頑張りましょう。 ***Reviewでの指摘 [#c7293d64] -私の修正は、自分自身でも、イマイチピンと来なかったのですが、指摘で --ニュートン法を繰り返すのはオーバースペック --11.054863929748535と11.054862976074219はhex表現で0x4130E0B9と0x4130E0B''9''で、そもそも1ULPSしか違わない --そんなbit exactに計算結果を比較しているのがナンセンス --そして、よく見れば等しいかどうかの比較において、doubleとして比較されていたのが原因 --という指摘がありました。 -私自身、PRを投げた経験と同じぐらい、この指摘で多くのことを学びました。 -つまり問題の根源は計算でなく、floatの比較にEXPECT''_DOUBLE_''EQというマクロを使ってることでした。 -なので、そこを適切に直して、私のPRは無事マージされました -ちなみにEXPECT_FLOAT_EQ内では、hex表現のfloatが、いくつ離れているか(ULPS)という点でtrueかfalseを返します。 -ちなみに、このhex表現のfloatがいくつ離れているか、というのは[[ULP>https://en.wikipedia.org/wiki/Unit_in_the_last_place]]という単位で表されます。 -丸め誤差を考慮し、最大でULP4つ分まで離れていても同じ浮動小数点である、と判定する仕組みになっています。 **マージ(Squash) [#a8f9d6d2] -Reviewや、BuildBotでの修正の過程で複数のcommitがpushされることがあります -通常、その過程を全て1つのcommitにまとめる(Squashする)ことが要求されます --branch元にgit reset --mixedする --pushした先のbranchをmasterにmergeする。その際、--squashオプションを付ける --再度commitする --再度pushする。この際、SHA1値が変わってるので、push --forceする * 感想 [#xaf58c3d] -テスト、すげー。そして大事 -一発でテストに通ると思わないほうが良い。テストでfailしたら恥ずかしいと思うけれど、恥じずに直す -何が問題なのか、ちゃんと説明できないなら、良く精査した方が良い *過去に投げたOpenCVのPull Request [#v68f1c35] [[Pull Requests · opencv/opencv>https://github.com/opencv/opencv/pulls?utf8=%E2%9C%93&q=is%3Apr+author%3Atomoaki0705]] [[:OpenCV]]
テキスト整形のルールを表示する
添付ファイル:
000-fork-and-pull-request.png
852件
[
詳細
]
000-fork.png
394件
[
詳細
]
001-clone.png
443件
[
詳細
]
002-modify.png
420件
[
詳細
]
003-push.png
411件
[
詳細
]
004-pullrequest.png
409件
[
詳細
]
005-pullrequest.png
422件
[
詳細
]
006-pullrequest-github.png
425件
[
詳細
]
![[PukiWiki]](image/pukiwiki.png "[PukiWiki]")
000-fork-and-pull-request.png 852件
[詳細]
000-fork.png 394件
[詳細]
001-clone.png 443件
[詳細]
002-modify.png 420件
[詳細]
003-push.png 411件
[詳細]
004-pullrequest.png 409件
[詳細]
005-pullrequest.png 422件
[詳細]
006-pullrequest-github.png 425件
[詳細]
