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

coordinatesクラスのx-axisメソッドの値がおかしい #468

Open
ban-masa opened this issue Nov 5, 2021 · 9 comments
Open

coordinatesクラスのx-axisメソッドの値がおかしい #468

ban-masa opened this issue Nov 5, 2021 · 9 comments

Comments

@ban-masa
Copy link

ban-masa commented Nov 5, 2021

(:x-axis () (matrix-row rot 0))
(:y-axis () (matrix-row rot 1))
(:z-axis () (matrix-row rot 2))

matrix-rowではなくmatrix-columnが正しいと思われます。

@k-okada
Copy link
Member

k-okada commented Nov 5, 2021 via email

@ban-masa
Copy link
Author

ban-masa commented Nov 9, 2021

単純にcoordinates.lを修正すればよいかと思っていたのですが、そうではないんでしょうか。

@k-okada
Copy link
Member

k-okada commented Nov 11, 2021

この手のバグが難しいのは,間違った仕様のまま使っているプログラムがあるケースです.
しかも,かなり根本的なところなので,気が付かない間にこれが変更されていてプログラムが動かくなった場合,なかなか気が付かないかと思います.

今どこで使われているか,というのは例えば以下のように探せるんだけど,これで大丈夫かどうかもちょっと心配.
https://github.com/search?q=org%3Ajsk-ros-pkg+%22%3Ax-axis%22&type=Code
https://github.com/search?q=org%3Astart-jsk+%22%3Ax-axis%22&type=code

例えば今回 @ban-masa はこのバグに気が付いた,ということなわけど,同じように気が付いたど,どこで使っているか第三者が調べられれば,逆に,ここで僕たちが調べて,使っている人に注意喚起できるのでよいんだけど...

で,現実的にはあまりそういうことはないけど,irtgeo.l で直すばあいは euslisp だけを使っている人には影響はないので,若干気が楽に自分で思いこめる,ということでした.なので,直すならcoordinates.l でいいんだけど,副作用の影響範囲をどう見積もるか,というのが悩みどころです.

まぁ,使っている人パッと見ていないから,エイ,と変えてしまうのでもいいんだけど..

@ban-masa
Copy link
Author

少なくとも今後axis系メソッドを使う人のためにもどちらかでは直しておきたいと思っています。
個人的には以下の理由でcoordinates.lを修正しても良いのではないかと考えています。

  1. irteusを使っている人のほうがかなり多いのではないかということ。
  2. x-axisを使っている人もバグに気づかず使っているのではないかということ(意図した挙動をしていないことに気づいていない可能性)
  3. バグに気づいた人はx-axisメソッドを使わないか自分で再定義しているであろうこと。
  4. 修正で実際に影響を受けるのはバグに気づいた上でx-axisメソッドの返り値を何かに使用している人であるが、回転行列の行ベクトルは使いみちに乏しい気がするので使ってる人はいないのではないかということ。

@YUKINA-3252
Copy link

@k-okada
こちらの問題なのですが、私もaxis系メソッドで得られる値を使いたいと思い、値が誤っていることに気づき、このissueで原因と経緯を知りました。私自身はメソッドをoverrideしました。
(このissueが立てられてから今まで他に言及されている方がいらっしゃらなかったので、新しくこのメソッドを使おうとする人があまりおらず、むしろ直すことで岡田先生がおっしゃるように間違った仕様のまま使っているプログラムに支障が出る影響のほうが大きい可能性のほうが高いと思いましたが一応の報告です。)

@Affonso-Gui
Copy link
Member

検索してみたけどヒットしたのは
https://github.com/jsk-ros-pkg/jsk_pr2eus/blob/master/pr2eus_tutorials/euslisp/template_grasp_samples.l#L261
だけでした。:y-axis:z-axisはヒット件数ゼロ。少なくとも他のベース関数に使われていることはなさそうです。

で、gitlabでも検索しようとしていたのですが、Advanced Searchができませんでした、、ライセンスはあると思うけど手動でenableしないといけない模様?
https://gitlab.jsk.imi.i.u-tokyo.ac.jp/help/user/search/advanced_search.md
https://docs.gitlab.com/ee/integration/advanced_search/elasticsearch.html#enable-advanced-search

@k-okada
Copy link
Member

k-okada commented Dec 10, 2022

@YUKINA-3252 おお,えらい.よく気付いたね.これが出てくるということは,結構細かく制御フローを作っていると思うので,フローチャートと,その時のx-axisの方向をirtviewerなりrvizで図示したものが作れそうな気がします.

@Affonso-Gui
https://github.com/search?p=4&q=org%3Ajsk-ros-pkg+%22%3Az-axis%22&type=Code
とか見ると古いeuslib時代のコードがおおそうですが,野田先生の時もまだ使っていそうな気もします.この時もeuslibだったのかな.ちなみにgitlabのはライセンスないとできないことはない?
image
AdvancedSearchの横の?を押すと以下になる.
https://gitlab.jsk.imi.i.u-tokyo.ac.jp/help/integration/elasticsearch#enable-advanced-search
会社としてはお金を払ってほしいから,無料の方法はひた隠しにするとは思うけど.
https://about.gitlab.com/pricing/
みると一人228USBなんだよね.何人使っていることになっているのかのカウントが良くわからないんだけど,アクティブに使っている人だけなら何とかなるかもしれない.LDAP見てアクセス可能な人,だとOBも残っていてかなり増えてきてしまいそう.
トライアルで試すのもありかもしれない.

@Affonso-Gui
Copy link
Member

なるほど、euslibにありましたか。

gitlabはアカデミックライセンスで使えることはないですか。
https://about.gitlab.com/solutions/education/
https://about.gitlab.com/handbook/marketing/community-relations/community-programs/education-program/

githubもbetaではliteral searchも追加しているらしいので、とりあえずwaitlistに入りました。
https://github.com/features/code-search

@Affonso-Gui
Copy link
Member

github search betaに入れました。
euslib, jsk_control, jsk-ros-pkg-unreleasedあたりに結構ありますね。

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

No branches or pull requests

4 participants