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

Naming: Clarify what "normalization" means #28

Open
mgeier opened this issue Dec 11, 2016 · 9 comments
Open

Naming: Clarify what "normalization" means #28

mgeier opened this issue Dec 11, 2016 · 9 comments

Comments

@mgeier
Copy link
Member

mgeier commented Dec 11, 2016

Currently we have sfs.util.normalize() and sfs.util.normal_vector() which do very different things.
This might be confusing.

@spors
Copy link
Member

spors commented Dec 12, 2016

The function sfs.util.normal_vector() takes a vector and normalizes it. We could therefore rename it to sfs.util.normalize_vector().

@trettberg
Copy link
Collaborator

Alternatively we could get rid of sfs.util.normal_vector(). It seems it is used only in sfs.util.rotation_matrix().

@mgeier
Copy link
Member Author

mgeier commented Dec 16, 2016

@trettberg That may be its only use for now, but we should call it wherever normal vectors are passed to functions, see #20.

@chris-hld
Copy link
Member

chris-hld commented Dec 16, 2016

I just stumbled across this problem.
I'll need a sfs.util.normalize_vector().
Please do not remove, but rename ;)

@hagenw
Copy link
Member

hagenw commented Dec 21, 2016

In #34 I introduced now util.normalize_vector(). I'm still not completely sure if this is the best name. Most of the time we are dealing with directions when we are using unit vectors. We have also util.direction_vector(alpha, beta=np.pi/2) and I could also imagine util.direction_vector(point1, point2). Maybe there is a way to combine all this?

@chris-hld
Copy link
Member

util.direction_vector(point1, point2) would be very handy, if not necessary.
Or I would introduce an optional "base" to normalize_vector() . The default would be 0.

Please have a look at this:

xs = np.r_[0.5, 0.5, 0] # position of virtual source
xref = np.r_[0, 0, 0]
nfs = sfs.util.normalize_vector(xref - xs)

For subtracting, I first have to create an array, which is not consistent with our array_like structure elsewhere. But actually I just have to specify the base of the vector to be xs.

Should we rather introduce a new function, or extend the existing?

@chris-hld chris-hld reopened this Dec 21, 2016
@mgeier
Copy link
Member Author

mgeier commented Dec 21, 2016

I agree that we need a way to subtract vectors (without having a vector data type), but I'm not sure if it is obvious enough what a function named direction_vector() with two points as input would do.

Does it just subtract or also normalize?

Also, Python doesn't do function overloading, so having the same direction_vector() function with alpha/beta and point1/point2 is a bit awkward.

The concept of a "base" in this context is new to me, I'm not sure if that's easy enough to understand.

What we need is something similar to a "look at" function.
Just some random links:
https://docs.unity3d.com/ScriptReference/Transform.LookAt.html
https://keithmaggio.wordpress.com/2011/01/19/math-magician-lookat-algorithm/

At some point, we might need a "up" vector, too. So it's probably good to think about that as well.

@chris-hld
Copy link
Member

Sorry, I think the accurate term is "initial point" of a vector.
Typically our direction vectors are normalized.

@mgeier
Copy link
Member Author

mgeier commented Jan 2, 2017

I still think it's a bit awkward to have this as an optional argument of normalize_vector().

Probably we shouldn't even use the word "vector", but instead something with the word "orientation" or "direction"?

I've created a separate issue for "view" and "up" vectors, which is probably related to this issue: #36.

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

5 participants