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.

Sign up for GitHub

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

Jump to bottom

Add angular momentum jacobian #98

Merged
merged 2 commits into from
May 10, 2016
Merged

Add angular momentum jacobian #98

merged 2 commits into from
May 10, 2016

Conversation

Tnoriaki
Copy link
Contributor

linkクラスに
慣性テンソル subIwを追加し
bodyクラスに
角運動量ヤコビアンを計算する関数calcAngularMomentum()
と
運動量ヤコビアンと角運動量ヤコビアンを使って重心周りの運動量と角運動量を計算する関数calcTotalMomentumFromJacobian()
を追加いたしました．

clacTotalMomentum()を使って求めた運動量，角運動量
と
calcTotalMomentFromJacobian()を使って求めた運動量,角運動量を原点周りに直した値
が一致することを確認しました．

@fkanehiro
Copy link
Owner

@k-okada さん、LinkやBodyが変わると下流も再コンパイルが必要になりますが、マージして大丈夫でしょうか。

@k-okada
Copy link
Contributor

k-okada commented Apr 26, 2016

先ずは，古いOpenHRP3が入っている人に，ソースを持って来てこパイルして下さい／最新のdebを持って来たください以外の解決策があるかないか調べて下さい　> @Tnoriaki

snozawa
snozawa reviewed Apr 26, 2016
View reviewed changes
hrplib/hrpModel/Body.cpp
Matrix33 Iw = rootLink_->subIw;
H.block(0, c+3, 3, 3) = Iw;
Vector3 cm = calcCM();
Matrix33 cm_cross;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

これと、D関数のように外積行列を計算するものは
https://github.com/fkanehiro/openhrp3/blob/master/hrplib/hrpUtil/Eigen3d.h#L80
で代用できそうでしょうか？

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

代用できます.
D関数を消し代わりにhat関数を用いてcommit致します.

@snozawa
Copy link
Contributor

snozawa commented Apr 26, 2016

@Tnoriaki

clacTotalMomentum()を使って求めた運動量，角運動量
と
calcTotalMomentFromJacobian()を使って求めた運動量,角運動量を原点周りに直した値
が一致することを確認しました．

の検算をするプログラムは、openhrp3にコミットできそうでしょうか？
それであれば、travis上でも走るように追加しておくと安心ですね。

@k-okada, @fkanehiro さん
今openhrp3でtravis上で走っている、基礎的なライブラリのテストを行ってる部分はございますでしょうか。

先ずは，古いOpenHRP3が入っている人に，ソースを持って来てこパイルして下さい／最新のdebを持って来たください以外の解決策があるかないか調べて下さい　> @Tnoriaki

@fkanehiroさん
こちらは、Link.hにsubIwというメンバ変数を追加する、ということをせずに、
単純にcalcXxx関数のみ機能追加としてたされるだけなら問題なさそうでしょうか。

@fkanehiro
Copy link
Owner

どこまで互換性を維持する必要があるのかは @k-okada さんに確認して頂いたほうがよいと思いますが、メンバ関数を追加するだけであれば、新しいhrpsys+古いopenhrp3でも（追加したものを呼ばない限りは）OKではないでしょうか。

@snozawa
Copy link
Contributor

snozawa commented Apr 26, 2016

どこまで互換性を維持する必要があるのかは @k-okada さんに確認して頂いたほうがよいと思いますが、メンバ関数を追加するだけであれば、新しいhrpsys+古いopenhrp3でも（追加したものを呼ばない限りは）OKではないでしょうか。

なるほど、ありがとうございます。

@Tnoriaki

こちらは、Link.hにsubIwというメンバ変数を追加する、ということをせずに、

メンバ変数の新規追加なしでPRの関数の計算できるかな？

@Tnoriaki
Copy link
Contributor Author

@nozawaさん
D関数を削除しメンバ変数の新規追加なしに修正いたしました.
openhrp3で検算するプログラムもつくります.

k-okada
k-okada reviewed Apr 27, 2016
View reviewed changes
hrplib/hrpModel/Link.h
@@ -59,7 +59,7 @@ namespace hrp {
@note assuming wc is already computed by Body::calcCM()
*/
void calcSubMassCM();

void calcSubMassInertia(Matrix33& subIw);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

他の関数と同じように関数のコメントをつけましょう

@Tnoriaki
Copy link
Contributor Author

Tnoriaki commented May 2, 2016

関数にコメントをつけ
不必要なdiffを消しました

@snozawa openhrp3で他に関数のテストをしているプログラムはなさそうなのですが
検算プログラムは無しでも大丈夫でしょうか？

@snozawa
Copy link
Contributor

snozawa commented May 6, 2016

関数にコメントをつけ
不必要なdiffを消しました

あれ、まだ不要な行変更ありませんか？
あと、そのあたりの整理がおわったら、コミットをsquashか何かでまとめるとよいと思います。
整理としては、コミットとして

  • Aという機能を追加しました
  • AにBというバグがあるので直しました、Cというタイポや不要な行があったので直しました。。。etc

があったときに、AのPR時期とBやCのPR時期が別な場合このコミットはあるものと思います。
一方、A、B、Cを同じPRに含めてコミットするときは、BやCが治ったものをAにsquashして、
あたかもバグがなくタイポや不要な行がなかったAが最初からコミットされてた
PRにしておくので良いと思います。

@snozawa openhrp3で他に関数のテストをしているプログラムはなさそうなのですが
検算プログラムは無しでも大丈夫でしょうか？

はい、他の関数も現状ないので、このコミット内では大丈夫そうに思います。

@Tnoriaki Tnoriaki force-pushed the add_angular_momentum_jacobian branch from c70fd61 to 1d6c67d Compare May 6, 2016 09:23
@Tnoriaki Tnoriaki force-pushed the add_angular_momentum_jacobian branch from 1d6c67d to 42761d7 Compare May 6, 2016 10:00
@Tnoriaki
Copy link
Contributor Author

Tnoriaki commented May 6, 2016

不必要な行変更を消しコミットをまとめました．

slackにてtravisのrestartをしようと思い
restart travis fkanehiro/openhrp3 187
を行うとrestart Botから
job 187(https://travis-ci.org/fkanehiro/openhrp3/jobs/128261508) has been restarted
とかえってくるのですが実際にはtravisがrestartされません.
どうすればいいでしょうか？

@snozawa
Copy link
Contributor

snozawa commented May 6, 2016

不必要な行変更を消しコミットをまとめました．

LGTM

@mmurooka
Copy link

mmurooka commented May 6, 2016

slackにてtravisのrestartをしようと思い
restart travis fkanehiro/openhrp3 187
を行うとrestart Botから
job 187(https://travis-ci.org/fkanehiro/openhrp3/jobs/128261508) has been restarted
とかえってくるのですが実際にはtravisがrestartされません.

slackからリスタートしてくれるスクリプトの
https://github.com/mmurooka/tools/blob/master/travis-bot/scripts/restart.coffee#L14
にopenhrp3も追加すればいいはずです．

@Tnoriaki Tnoriaki force-pushed the add_angular_momentum_jacobian branch from 42761d7 to f5f65b8 Compare May 7, 2016 08:04
@Tnoriaki
Copy link
Contributor Author

Tnoriaki commented May 7, 2016

@mmurookaさんありがとうございます.
デフォルトでは使えないのですね.

@Tnoriaki Tnoriaki mentioned this pull request May 9, 2016
Open
@Tnoriaki
Copy link
Contributor Author

指摘部分を修正しtravisも通りましたので
もしよろしければmergeしていただけると幸いです.

@fkanehiro fkanehiro merged commit cf77482 into fkanehiro:master May 10, 2016
@Tnoriaki Tnoriaki deleted the add_angular_momentum_jacobian branch May 10, 2016 12:52
@k-okada
Copy link
Contributor

k-okada commented Feb 17, 2017

@fkanehiro @snozawa @Tnoriaki
これ、古いOpenHRP3のmodelloaderを、新しいOpenHRP3でコンパイルされたhrpsysから読みに行っても大丈夫なのかな？idlが変わっていない限り大丈夫？

@fkanehiro
Copy link
Owner

それは問題ないと思います。

@kyawawa kyawawa mentioned this pull request Jun 27, 2017
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Tnoriaki @fkanehiro @k-okada @snozawa @mmurooka