OpenCVのバグ(Pull Request)
をテンプレートにして作成
[
トップ
] [
新規
|
一覧
|
検索
|
最終更新
|
ヘルプ
]
開始行:
[[OpenCVのバグ(trac)]]
この度、OpenCVへのPull Request がマージされたので、その時...
* Itroduction [#p200d0e4]
-First, always check the https://github.com/opencv/opencv...
* 時系列 [#q5b6e51d]
** testのfailに気づく [#h4d00b0d]
-ことの始まりは[[ODROID-C2]]を買って、その上でOpenCVのtes...
-何故か、testの一部がFAILEDとなる
-見てみると、hal_intrinというテストが失敗していた
-具体的には、HW命令を使用して平方根(square root)を求めた...
/home/odroid/opencv-fork/modules/core/test/test_intrin.c...
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:話...
** エラーしている箇所を眺めてみる [#qf81f66d]
-内容は、11.054862976074219になるべき結果が、
-なぜか、11.05486''3929748535''になっているというエラー
-もともとも、HW命令を使った計算では、[[ニュートン法>メモ#...
-反復することで、精度があがるので、まぁ、もう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...
-ちなみに、ArmのNEON命令を使っているので、float4つ分が一...
-ベクタの要素のうち1つが、計算結果がズレ過ぎている、とし...
** 修正案を実装してPR(Pull Request)する [#n6a10b6c]
-試しに実装したらテストのFAILEDがPASSした
-なので、1行だけの修正だし、これで良かれと思って、PRを投...
*** How to contributeページを読む [#q4e09cbe]
-ここからが大事なところ。OpenCVでは、[[How to contirbute>...
-ここでは、PRを投げる前に読むべき大事な点がたくさん書かれ...
-Gitのお作法だけでなく、コーディングスタイルに関しても書...
-そして、なるだけ、機械的なチェックをパスするように指導さ...
-''注意'' ここで書いている情報は2016-05-16時点の情報なの...
*** Githubのお作法 [#h46e1c5b]
-この節だけはOpenCV以外のプロジェクトにも共通する、gitのf...
-手順としては
#ref(000-fork-and-pull-request.png)
+opencv/opencv リポジトリを自身のリポジトリに fork する
+自身のリポジトリから、ローカルに clone する
+ローカルでfixして、commitする
+自身のリポジトリにpushする
+GithubのインタフェースからPull Request を作る
-という感じです。詳細は[[githubの説明(リポジトリをforkす...
*** 使用するブランチ [#q2b90933]
-以降の節はOpenCVに限った話
-OpenCVは2016年5月16日現在、3.0系列と2.4系列の2本の系列が...
--OpenCV2.4系列は既に「新機能の追加は行わない」とアナウン...
--OpenCV3.0系列がメインの開発対象となっており、新規機能は...
-なので、bug fix ならば2.4系列、もしくは両方に、新規機能...
-今回私が投げたPRはbug fixなので、本来は2.4系列にも投げる...
*** pre-commit hook [#c409f59e]
-前述の手順でローカルにリポジトリをcloneした後、作業をす...
-具体的には、cloneしたリポジトリの .git/hooks/pre-commit....
-この.gitディレクトリは通常隠しディレクトリなので、うまい...
-これをすることで、White spce に関する規約をcommit 直前に...
--ちなみに、OpenCVはインデントはスペース4つです。タブは使...
-例えばインデントにタブを使うと、Commit自体がエラーになり...
**PRを書くコツ [#h311d670]
-最近、PRを書く際に、テンプレートが表示されるようになりま...
-テンプレートには、大きく、**What does this PR change **...
-ここに、分かりやすく
--このPRで何をしたいのか
--このPRを何故書いたのか
--などを分かりやすく説明します
-私のオススメとしては、この説明文を上手く書けないようなPR...
**BuildBot [#eabbfe16]
-PRを投げたあとの全体的な流れは[[この図>http://code.openc...
-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もなく、エラーもなく、その上...
-ただし、マージされるかどうかは、Reviewが入ります
**Review [#cd975f57]
-Reviewのためには、Intelのメンバが承認することが必要です。
-マージする前に直すべき点、コードの書き方、テストが抜けて...
-ReviewでOKが出ると、Intelのメンバがアサインされます
-アサインされたメンバが絵文字のサムアップ¬e{thumb-up:...
-当然英語でやり取りをする必要があります。頑張りましょう。
***Reviewでの指摘 [#c7293d64]
-私の修正は、自分自身でも、イマイチピンと来なかったのです...
--ニュートン法を繰り返すのはオーバースペック
--11.054863929748535と11.054862976074219はhex表現で0x4130...
--そんなbit exactに計算結果を比較しているのがナンセンス
--そして、よく見れば等しいかどうかの比較において、double...
--という指摘がありました。
-私自身、PRを投げた経験と同じぐらい、この指摘で多くのこと...
-つまり問題の根源は計算でなく、floatの比較にEXPECT''_DOUB...
-なので、そこを適切に直して、私のPRは無事マージされました
-ちなみにEXPECT_FLOAT_EQ内では、hex表現のfloatが、いくつ...
-ちなみに、このhex表現のfloatがいくつ離れているか、という...
-丸め誤差を考慮し、最大でULP4つ分まで離れていても同じ浮動...
**マージ(Squash) [#a8f9d6d2]
-Reviewや、BuildBotでの修正の過程で複数のcommitがpushされ...
-通常、その過程を全て1つのcommitにまとめる(Squashする)こ...
--branch元にgit reset --mixedする
--pushした先のbranchをmasterにmergeする。その際、--squash...
--再度commitする
--再度pushする。この際、SHA1値が変わってるので、push --fo...
* 感想 [#xaf58c3d]
-テスト、すげー。そして大事
-一発でテストに通ると思わないほうが良い。テストでfailした...
-何が問題なのか、ちゃんと説明できないなら、良く精査した方...
*過去に投げたOpenCVのPull Request [#v68f1c35]
[[Pull Requests · opencv/opencv>https://github.com/o...
[[:OpenCV]]
終了行:
[[OpenCVのバグ(trac)]]
この度、OpenCVへのPull Request がマージされたので、その時...
* Itroduction [#p200d0e4]
-First, always check the https://github.com/opencv/opencv...
* 時系列 [#q5b6e51d]
** testのfailに気づく [#h4d00b0d]
-ことの始まりは[[ODROID-C2]]を買って、その上でOpenCVのtes...
-何故か、testの一部がFAILEDとなる
-見てみると、hal_intrinというテストが失敗していた
-具体的には、HW命令を使用して平方根(square root)を求めた...
/home/odroid/opencv-fork/modules/core/test/test_intrin.c...
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:話...
** エラーしている箇所を眺めてみる [#qf81f66d]
-内容は、11.054862976074219になるべき結果が、
-なぜか、11.05486''3929748535''になっているというエラー
-もともとも、HW命令を使った計算では、[[ニュートン法>メモ#...
-反復することで、精度があがるので、まぁ、もう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...
-ちなみに、ArmのNEON命令を使っているので、float4つ分が一...
-ベクタの要素のうち1つが、計算結果がズレ過ぎている、とし...
** 修正案を実装してPR(Pull Request)する [#n6a10b6c]
-試しに実装したらテストのFAILEDがPASSした
-なので、1行だけの修正だし、これで良かれと思って、PRを投...
*** How to contributeページを読む [#q4e09cbe]
-ここからが大事なところ。OpenCVでは、[[How to contirbute>...
-ここでは、PRを投げる前に読むべき大事な点がたくさん書かれ...
-Gitのお作法だけでなく、コーディングスタイルに関しても書...
-そして、なるだけ、機械的なチェックをパスするように指導さ...
-''注意'' ここで書いている情報は2016-05-16時点の情報なの...
*** Githubのお作法 [#h46e1c5b]
-この節だけはOpenCV以外のプロジェクトにも共通する、gitのf...
-手順としては
#ref(000-fork-and-pull-request.png)
+opencv/opencv リポジトリを自身のリポジトリに fork する
+自身のリポジトリから、ローカルに clone する
+ローカルでfixして、commitする
+自身のリポジトリにpushする
+GithubのインタフェースからPull Request を作る
-という感じです。詳細は[[githubの説明(リポジトリをforkす...
*** 使用するブランチ [#q2b90933]
-以降の節はOpenCVに限った話
-OpenCVは2016年5月16日現在、3.0系列と2.4系列の2本の系列が...
--OpenCV2.4系列は既に「新機能の追加は行わない」とアナウン...
--OpenCV3.0系列がメインの開発対象となっており、新規機能は...
-なので、bug fix ならば2.4系列、もしくは両方に、新規機能...
-今回私が投げたPRはbug fixなので、本来は2.4系列にも投げる...
*** pre-commit hook [#c409f59e]
-前述の手順でローカルにリポジトリをcloneした後、作業をす...
-具体的には、cloneしたリポジトリの .git/hooks/pre-commit....
-この.gitディレクトリは通常隠しディレクトリなので、うまい...
-これをすることで、White spce に関する規約をcommit 直前に...
--ちなみに、OpenCVはインデントはスペース4つです。タブは使...
-例えばインデントにタブを使うと、Commit自体がエラーになり...
**PRを書くコツ [#h311d670]
-最近、PRを書く際に、テンプレートが表示されるようになりま...
-テンプレートには、大きく、**What does this PR change **...
-ここに、分かりやすく
--このPRで何をしたいのか
--このPRを何故書いたのか
--などを分かりやすく説明します
-私のオススメとしては、この説明文を上手く書けないようなPR...
**BuildBot [#eabbfe16]
-PRを投げたあとの全体的な流れは[[この図>http://code.openc...
-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もなく、エラーもなく、その上...
-ただし、マージされるかどうかは、Reviewが入ります
**Review [#cd975f57]
-Reviewのためには、Intelのメンバが承認することが必要です。
-マージする前に直すべき点、コードの書き方、テストが抜けて...
-ReviewでOKが出ると、Intelのメンバがアサインされます
-アサインされたメンバが絵文字のサムアップ¬e{thumb-up:...
-当然英語でやり取りをする必要があります。頑張りましょう。
***Reviewでの指摘 [#c7293d64]
-私の修正は、自分自身でも、イマイチピンと来なかったのです...
--ニュートン法を繰り返すのはオーバースペック
--11.054863929748535と11.054862976074219はhex表現で0x4130...
--そんなbit exactに計算結果を比較しているのがナンセンス
--そして、よく見れば等しいかどうかの比較において、double...
--という指摘がありました。
-私自身、PRを投げた経験と同じぐらい、この指摘で多くのこと...
-つまり問題の根源は計算でなく、floatの比較にEXPECT''_DOUB...
-なので、そこを適切に直して、私のPRは無事マージされました
-ちなみにEXPECT_FLOAT_EQ内では、hex表現のfloatが、いくつ...
-ちなみに、このhex表現のfloatがいくつ離れているか、という...
-丸め誤差を考慮し、最大でULP4つ分まで離れていても同じ浮動...
**マージ(Squash) [#a8f9d6d2]
-Reviewや、BuildBotでの修正の過程で複数のcommitがpushされ...
-通常、その過程を全て1つのcommitにまとめる(Squashする)こ...
--branch元にgit reset --mixedする
--pushした先のbranchをmasterにmergeする。その際、--squash...
--再度commitする
--再度pushする。この際、SHA1値が変わってるので、push --fo...
* 感想 [#xaf58c3d]
-テスト、すげー。そして大事
-一発でテストに通ると思わないほうが良い。テストでfailした...
-何が問題なのか、ちゃんと説明できないなら、良く精査した方...
*過去に投げたOpenCVのPull Request [#v68f1c35]
[[Pull Requests · opencv/opencv>https://github.com/o...
[[:OpenCV]]
ページ名:
![[PukiWiki]](image/pukiwiki.png "[PukiWiki]")
