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

Fix non-unit direction vectors #34

Merged
merged 2 commits into from
Jan 11, 2017
Merged

Fix non-unit direction vectors #34

merged 2 commits into from
Jan 11, 2017

Conversation

hagenw
Copy link
Member

@hagenw hagenw commented Dec 21, 2016

This tries to fix #20 and #28.

I renamed util.normal_vector() to util.normalize_vector() as proposed in #28 and used it for every single dimensional direction vector n. 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).

@mgeier
Copy link
Member

mgeier commented Jan 3, 2017

Any objections?
If not, LGTM!

@hagenw
Copy link
Member Author

hagenw commented Jan 3, 2017

Just a reminder, the fix is not done for n0 = util.asarray_of_rows(n0) as I didn't know what is the best way to apply it there. With a loop? Can someone help out with this?

@mgeier
Copy link
Member

mgeier commented Jan 3, 2017

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 util.normalize_vector()?
We could do a simple np.asarray() and then do different things for one and two-dimensional arrays.
This could be used like util.normalize_vector(util.asarray_of_rows(n0))

Or should we make a new function util.asarray_of_normalized_rows()?
This has the problem of #28, could be both types of "normalization".

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 keepdims argument is only available since NumPy 1.10.0 (see https://docs.scipy.org/doc/numpy/reference/generated/numpy.linalg.norm.html).

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)

@mgeier
Copy link
Member

mgeier commented Jan 3, 2017

PS: if we include this in util.normalize_vector(), it will lose its squeezing ability.
This means we would have to use util.normalize_vector(util.asarray_1d(n)) in many cases.

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 normalize_vector() and normalize_vectors().

BTW, if we decide for some fundamental changes along the lines of #36, we'll have to change those functions again.

@mgeier
Copy link
Member

mgeier commented Jan 3, 2017

PPS: We can also implement this later, we should just create a new issue as a reminder.

@hagenw
Copy link
Member Author

hagenw commented Jan 4, 2017

At the beginning I was also thinking of two functions similar to the asarray once, e.g.

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))

@mgeier
Copy link
Member

mgeier commented Jan 10, 2017

I would suggest to merge this PR as is and postpone further changes until we see how our understanding of a "direction" evolves.

@hagenw
Copy link
Member Author

hagenw commented Jan 10, 2017

Fine with me, I created an issue for the remaining TODO of this PR.

@mgeier mgeier merged commit 8a97603 into master Jan 11, 2017
@mgeier mgeier deleted the direction_vectors branch January 11, 2017 13:15
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

Successfully merging this pull request may close these issues.

Handling of directional vector for plane waves
2 participants