euslisp / EusLisp

EusLisp is an integrated programming system for the research on intelligent robots based on Common Lisp and Object-Oriented programming. [Manual](http://euslisp.github.io/EusLisp/manual.html ) [マニュアル](http://euslisp.github.io/EusLisp/jmanual.html )
Other
56 stars 50 forks source link

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

Open ban-masa opened 2 years ago

ban-masa commented 2 years ago

https://github.com/euslisp/EusLisp/blob/e33942c6975a6f426b987f584479a3e31b6ff98c/lisp/l/coordinates.l#L51-L53 matrix-rowではなくmatrix-columnが正しいと思われます。

k-okada commented 2 years ago

なるほど,だから https://github.com/jsk-ros-pkg/euslib/blame/master/jsk/jskgeo.l#L493-L495 があったのか.

これ,どうしようか.irtgeo.lに追加するのが,正しかったんだろうけど,今まで使った人はいないのかな.

◉ Kei Okada

2021年11月5日(金) 19:23 Bando Masahiro @.***>:

https://github.com/euslisp/EusLisp/blob/e33942c6975a6f426b987f584479a3e31b6ff98c/lisp/l/coordinates.l#L51-L53 matrix-rowではなくmatrix-columnが正しいと思われます。

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

ban-masa commented 2 years ago

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

k-okada commented 2 years ago

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

今どこで使われているか,というのは例えば以下のように探せるんだけど,これで大丈夫かどうかもちょっと心配. 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 commented 2 years ago

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

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

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

Affonso-Gui commented 1 year ago

検索してみたけどヒットしたのは 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 commented 1 year ago

@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 commented 1 year ago

なるほど、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 commented 1 year ago

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