-
Notifications
You must be signed in to change notification settings - Fork 20
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
Fix non-unit direction vectors #34
Conversation
Any objections? |
Just a reminder, the fix is not done for |
If this would be needed only once, you could do something like: n0 = util.asarray_of_rows(n0)
n0 = np.row_stack([util.normalize_vector(n) for n in n0]) But since we need this several times, we should make a function. Should we include this into Or should we make a new function In a separate function, you could use something like this: # make sure x is a 2-dimensional array
x / np.linalg.norm(x, axis=1, keepdims=True) However, note that the This should do the same thing and also work for older versions: # make sure x is a 2-dimensional array
x / np.linalg.norm(x, axis=1).reshape(-1, 1) |
PS: if we include this in Which would be a bit longer, but it would actually be more explicit, which generally is a good thing. Another problem: we would have to decide between BTW, if we decide for some fundamental changes along the lines of #36, we'll have to change those functions again. |
PPS: We can also implement this later, we should just create a new issue as a reminder. |
At the beginning I was also thinking of two functions similar to the util.asdirection()
util.asdirections() But maybe it is better to have to use it in an explicit way? util.normalize_vector(util.asarray_1d(n))
util.normalize_vector(util.asarray_of_rows(n)) |
I would suggest to merge this PR as is and postpone further changes until we see how our understanding of a "direction" evolves. |
Fine with me, I created an issue for the remaining TODO of this PR. |
This tries to fix #20 and #28.
I renamed
util.normal_vector()
toutil.normalize_vector()
as proposed in #28 and used it for every single dimensional direction vectorn
. The field plotted in #20 looks now correct with this.I'm still not happy with the naming, which I will discuss directly in #28 and there is the problem how to handle several unit vectors at once like
n0 = util.asarray_of_rows(n0)
.