Skip to content

[lisp/geo/geopack.l] normalize norm vector in optional arugment of vector-angle function. (Improved version of #262) #264

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
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[lisp/geo/geopack.l] normalize norm vector in optional arugment of vector-angle function. (Improved version of #262) #264

wants to merge 1 commit into from

Conversation

mmurooka
Copy link
Member

This is improved version of #262 according to the comments of #262 (comment) and 038d5bc#commitcomment-27667058 .

@mmurooka mmurooka mentioned this pull request Feb 20, 2018
Merged
@k-okada
Copy link
Member

k-okada commented Feb 28, 2018 •
edited
Loading

how do you think we we move https://github.com/euslisp/jskeus/blob/master/irteus/irtmath.l#L610-L620. to EusLisp?

Basically it is not good idea to change existing behavior #257 (comment)

but...

  1. most of user uses irteus and they use modified normalize-vector interface.
  2. the behavior of 0 input is not defined http://euslisp.github.io/EusLisp/jmanual-node16.html#SECTION03031000000000000000
  3. sklearn example (https://stackoverflow.com/questions/21030391/how-to-normalize-an-array-in-numpy )
>>> from sklearn.preprocessing import normalize
>>> normalize([[1,2,3]])
array([[ 0.26726124,  0.53452248,  0.80178373]])
>>> normalize([[0,0,0]])
array([[ 0.,  0.,  0.]])

@mmurooka
Copy link
Member Author

how do you think we we move https://github.com/euslisp/jskeus/blob/master/irteus/irtmath.l#L610-L620. to EusLisp?

As far as I know, it's no problem. Are there any application which uses raw eus instead of irteus?

@mmurooka
Copy link
Member Author

mmurooka commented Feb 28, 2018 •
edited
Loading

most of user uses irteus and they use modified normalize-vector interface.

Geometry classes defined in euslisp are most important users of normalized-vector defined in eus.

Following many source codes use normalized-vector defined in eus. It's difficult to check all effects of changing normalized-vector... So, I think keeping current definition will be safe.

leus@cygnus:~/ros/indigo_parent/src/euslisp/Euslisp$ find ./ -name "*.l" | xargs grep normalize-vector
./lib/demo/hehehe.l:	(setq pln (make-plane :normal (normalize-vector #f(1 2 8))
./lib/demo/meteor.l:     (normalize-vector
./lib/demo/kogi.l:	(setq pln (make-plane :normal (normalize-vector #f(1 2 8))
./lib/llib/eusdraw.l:			 (normalize-vector (rotate-vector a 0.5 nil))))
./lib/llib/eusdraw.l:			 (normalize-vector (rotate-vector a -0.5 nil))))
./lib/llib/eusdraw.l:			 (normalize-vector (rotate-vector a 0.5 nil))))
./lib/llib/eusdraw.l:			 (normalize-vector (rotate-vector a -0.5 nil))))
./lib/llib/utyo.l:	      (normalize-vector (v* (v- p3 p2)
./lib/llib/quaternion.l:		ss   (scale (sqrt (- 1.0 (* ss ss))) (normalize-vector aa))))
./lib/llib/quaternion.l: (:axis () (normalize-vector a))
./contrib/utyo/glue.l:  (let* ((v1 (normalize-vector (v- (edg . pvert) (edg . nvert))))
./contrib/utyo/glue.l:     (setq ang (vector-angle (normalize-vector (v- p1 int-point))
./contrib/utyo/glue.l:                             (normalize-vector (v- p2 int-point))))
./contrib/utyo/glue.l:       (normalize-vector 
./contrib/utyo/glue.l:          (normalize-vector 
./contrib/utyo/part.l:            (norm (normalize-vector 
./contrib/utyo/linraphs.l:	 (r3 (normalize-vector (v* r1 r2))))
./contrib/utyo/linraphs.l:    (v* r3 (normalize-vector r1) r2)
./contrib/utyo/local.l:	       (normalize-vector
./contrib/utyo/newton.l:	 (r3 (normalize-vector (v* r1 r2))))
./contrib/utyo/newton.l:    (v* r3 (normalize-vector r1) r2)
./contrib/utyo/round.l:          (n-vect (normalize-vector vect))
./contrib/utyo/round.l:             (norm (normalize-vector
./contrib/utyo/round.l:             (norm (normalize-vector
./contrib/utyo/round.l:             (norm (normalize-vector
./contrib/utyo/localop.l:            (norm (normalize-vector 
./contrib/contact/model2constRobust.l:			      :nvertex (normalize-vector (scale sign n-point)) 
./lisp/geo/geopack.l:  (normalize-vector (v- dest org)))
./lisp/geo/geopack.l:   (normalize-vector (v* (v- b a) (v- c a))))
./lisp/geo/geopack.l:(defun vector-angle (v1 v2 &optional (normal (normalize-vector (v* v1 v2))) (parallel-thre 1e-10))
./lisp/geo/geopack.l:       (normalize-vector normal normal))      )
./lisp/geo/geopack.l:   (normalize-vector (float-vector (random 1.0) (random 1.0) (random 1.0))))
./lisp/geo/geopack.l: (:direction () (normalize-vector (v- nvert pvert)))
./lisp/geo/geopack.l:    (cond ((eq f pface) (normalize-vector (v- nvert pvert)))
./lisp/geo/geopack.l:	  ((eq f nface) (normalize-vector (v- pvert nvert)))
./lisp/geo/geopack.l:     (normalize-vector (v*  (cond ((eq f pface) (v- nvert pvert))
./lisp/geo/geopack.l:			     (normalize-vector (v- nvert pvert)))))
./lisp/geo/geopack.l:   (let* ((z (normalize-vector (v- nvert pvert)))
./lisp/geo/geopack.l:;	  (setq light-source (normalize-vector
./lisp/geo/geopack.l:      (if (minusp (* sign1 sign2)) (normalize-vector (scale sign1 normal)))))
./lisp/geo/geopack.l:  (setq normal (normalize-vector normal))
./lisp/geo/gdome.l:	(normalize-vector gvector gvector)
./lisp/geo/viewing.l:	(setq view-direction (normalize-vector view-direction))
./lisp/geo/viewing.l:	(setq view-right (normalize-vector view-right))
./lisp/geo/viewing.l:	(setq view-up (normalize-vector (v* view-right view-direction)))
./lisp/geo/viewing.l:     (normalize-vector
./lisp/geo/viewing.l:      (setq v1 (normalize-vector (send box :minpoint))
./lisp/geo/viewing.l:	    v2 (normalize-vector (send box :maxpoint)))
./lisp/geo/viewing.l:              (normalize-vector
./lisp/geo/lighting.l:     (normalize-vector
./lisp/geo/render.l:     (normalize-vector
./lisp/geo/render.l:	      (normalize-vector normal-view normal-view)
./lisp/geo/render.l:		   (normalize-vector lighting normal-lighting)
./lisp/geo/extroid.l:   (normalize-vector (float-vector (random 1.0) (random 1.0))))
./lisp/geo/geobody.l:			    (t (normalize-vector axis))))
./lisp/geo/geobody.l:	(scale radius (normalize-vector (v- point (send self :worldpos))))))
./lisp/geo/grahamhull.l:	  (vstart (normalize-vector (v- start o)))
./lisp/geo/grahamhull.l:	 (normalize-vector vtemp vtemp)
./lisp/geo/primt.l:	  (n (normalize-vector (v* v1 v2))))
./lisp/geo/primt.l:                   (vector-angle (normalize-vector vec)
./lisp/geo/primt.l:                                 (normalize-vector fvec)
./lisp/geo/primt.l:                                 (normalize-vector (v* vec fvec))
./lisp/geo/primt.l:                                       (normalize-vector n)
./lisp/geo/primt.l:                                       (normalize-vector (v* #f(0 0 1) n))
./lisp/geo/primt.l:	 (setq v (scale radius (normalize-vector (v+ (e . pvert) (e . nvert)))))

@k-okada
Copy link
Member

k-okada commented Feb 28, 2018 via email

@k-okada
Copy link
Member

k-okada commented Apr 4, 2018

@mmurooka

Following many source codes use normalized-vector defined in eus. It's difficult to check all effects of changing normalized-vector... So, I think keeping current definition will be safe.

can you check more detail of how they uses 'normalize-vector' using 'grep -A' ?

@mmurooka
Copy link
Member Author

mmurooka commented Apr 4, 2018

https://gist.github.com/mmurooka/282f7aec8dba5ddc7630a92849dced12 is result of grep.
Maybe there is no problem but it's very difficult for me to be confident that there is no secondary effect.

For example, if some user writes only the following simple code, behavior changes.

(setq tmp-edge (make-line #f(0 0 0) #f(0 0 0)))
(when (equal *nan* (norm (send tmp-edge :direction)))
  ...)
;; becomes true in current behavior but becomes false if we change normalize-vector behavior.

@k-okada k-okada mentioned this pull request Apr 8, 2018
Open
@k-okada
Copy link
Member

k-okada commented Apr 8, 2018

Sorry that I'm confused, this PR does not change the behavior of normalize-vector nor norm, it changes vector-angle implementation. If this is correct, we'd like to check who is using this function, including jsk-ros-pkg, start-jsk and euslib

@k-okada
Copy link
Member

k-okada commented Jul 11, 2018

@mmurooka can you check 'norm' use cases?

@mmurooka
Copy link
Member Author

this PR does not change the behavior of normalize-vector nor norm, it changes vector-angle implementation. If this is correct, we'd like to check who is using this function, including jsk-ros-pkg, start-jsk and euslib

Yes, this PR does NOT change the behavior of normalize-vector nor norm. This PR fixes the following bug of vector-angle.

1.irteusgl$ (vector-angle #f(1 0 0) #f(1 1 0))
0.785398
2.irteusgl$ (vector-angle #f(2 0 0) #f(1 1 0))
1.10715 ;; result does not same !!!

Sorry but what is the purpose of check 'norm' use cases?

@k-okada
Copy link
Member

k-okada commented Jul 11, 2018 •
edited
Loading

Sorry this is my misunderstanding

+    (if (or (= (norm normal) 0.0) ;; for irteus
+            (equal (norm normal) *nan*)) ;; for eus

I thought norm was changed, but https://github.com/euslisp/jskeus/blob/master/irteus/irtmath.l#L610-L620 changes normalize-vector behavior

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
PR/WaitForFeedback question
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mmurooka @k-okada