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

Functions for normalizing vectors #12047

Closed
c42f opened this issue Jul 7, 2015 · 22 comments
Closed

Functions for normalizing vectors #12047

c42f opened this issue Jul 7, 2015 · 22 comments
Labels

Comments

@c42f
Copy link
Member

c42f commented Jul 7, 2015

I find myself commonly writing the trivial utility function normalize(x) = x/norm(x) for geometric computing. Julia has the useful utility cross() in linalg - maybe normalize() and normalize!() belong there too? I see that ImmutableArrays.jl has defined unit() for this though that name clashes with a different use in SIUnits.

For greater than 1D arrays, normalize(x, dim) acting along a dimension would be a useful generalization for me personally. A p-norm version might also be useful.

@SimonDanisch - this is very relevant to 3D graphics - maybe you have some thoughts?

@andreasnoack
Copy link
Member

I agree that it is a useful function, but it is also very easy to define in a program, so I think I prefer not to include it. I'm aware that we might be deviating from this principle for other functions in linalg.

@tknopp
Copy link
Contributor

tknopp commented Jul 7, 2015

+1 for adding this to base. The actual implementation should however not look like normalize(x) = x/norm(x) but avoid the extra temporary.

@c42f
Copy link
Member Author

c42f commented Jul 7, 2015

I was imagining the actual implementation would be normalize(x) = normalize!(copy(x)), with normalize! a suitable zero-allocation version. My usual preferred name for the non-inplace version is normalized(), but I thought normalize() and normalize!() might be more idiomatic?

Out of curiosity, how is an extra temporary involved in x/norm(x)? If you don't want normalization to be in place you're bound to have at least one copy.

@andreasnoack - understood, though I'd argue that normalizing a vector is pretty fundamental to all sorts of numerical linear algebra. In addition, a maximally efficient version of the generalized version normalize(x, dim) is not trivial to implement. (I'd guess that ordering based on memory layout and simd instruction usage would be relevant. That's not something your average user is going to bother with, but it could eventually make sense to include.)

@tknopp
Copy link
Contributor

tknopp commented Jul 7, 2015

Ah sorry you are right. In this case its just the one necessary copy being made.

@carnaval
Copy link
Contributor

carnaval commented Jul 7, 2015

For the 3D case I don't think anybody would use julia arrays for a single vec3 (or projective vec4). It might come up however to have to project on the sphere a whole bunch of them represented as a big array (say if you want to avoid renormalizing normals on the GPU, even though it's pretty cheap these days) but in general the small vectors in the array will have stride + offset.

@timholy
Copy link
Sponsor Member

timholy commented Jul 7, 2015

normalize(x, dim) is just x./sqrt(sumabs2(x, dim)), and it already uses a cache-efficient simdable algorithm. Overall I'm 👎 on including this in base, in no small measure because deciding the proper behavior for when x has zero norm could be the subject of a long-running discussion (wasting much more time than if people just implement whatever behavior they want for themselves).

@tknopp
Copy link
Contributor

tknopp commented Jul 7, 2015

Why would this lead to long discussions? Either throw an exception (like we do for sqrt(-1)) or in the lazy implementation simple do nothing. I think we have more exotic functions in base than normalize.

@c42f
Copy link
Member Author

c42f commented Jul 7, 2015

@carnaval - I commonly use julia arrays for a single vec3 - they are a reasonable tool when you want the shortest/simplest code possible and efficiency isn't a concern.

@johnmyleswhite
Copy link
Member

Agree with @timholy: there's no reason this needs to be in Base.

@nalimilan
Copy link
Member

One reason is convenience: you can use that function without having to define it somewhere first. The other is consistency: if each project ends up implementing a slightly different version, you'll never know how it exactly behaves, e.g. when the norm is zero.

After all, we already have scale.

@c42f
Copy link
Member Author

c42f commented Jul 7, 2015

Tim - that's great, I didn't know about sumabs2.

@ScottPJones
Copy link
Contributor

@nalimilan So maybe a bunch of these "convenience" functions belong in a registered package, that would solve both your convenience and consistency issues, IMO, and not keep adding more and more to base.

@tknopp
Copy link
Contributor

tknopp commented Jul 7, 2015

I am a big fan of having a shrinked base. But this does not mean that new functions are disallowed. Linear algebra lives in base and one should try to group function together that belong to a functionality.
But I am not so much arguing for this specific function (normalize) but more so about @nalimilan comment. If a function is repeated a lot in user code with slightly different versions its time to
a) have a package
b) put it in base
If its not a single function its good to have a functional group and do a) But if all the other functions are already in base it should also be valid to do b).

(The function squeeze is by the way a similar candidate that I would assume is repeated by various people and despite its issues could live in base simple because it is used by several people)

@dhoegh
Copy link
Contributor

dhoegh commented Jul 7, 2015

I have also used and defined it in some projects, +1 for having it in base.

@stevengj stevengj added the domain:linear algebra Linear algebra label Jul 7, 2015
@SimonDanisch
Copy link
Contributor

I'd also like to have it in base for the reasons already mentioned.
I think there is no doubt about it being a widely used operation.
When I implemented FixedSizeArrays, it felt fairly odd to have so many functions defined on arrays in Base, but not normalize.
Its pretty much the only function I export in FixedSizeArrays, besides some constructors.
When solving #11610 we can inherit from AbstractArray for FixedSizeArrays, which would make it even weirder to sneak in normalize over some other package.
I wouldn't count the argument, that its really easy to implement.
There are lots of easy to implement functions in Base, but they're still in there as they're considered to be widely used.

@PaulBellette
Copy link

+1 I use it all the time

@josephnunn
Copy link

Although it may not seem like a big deal to everyone, for people working in certain areas like graphics or machine learning not having a normalize function in base is quite frankly ridiculous given all the linear algebra functionality already available. In these problem areas normalize's usage is as prevalent as the veritable List class is in other programming tasks. And for those languages that don't have List functionality, you'll always find a buggy or unoptimized homespun class hanging around the code base causing problems.

As to the general size of base, Common Lisp showed that any sufficiently developed language will in the end absorb a large number of functions into the basic language, its unavoidable and in my opinion preferable to having a large number of ill conceived or maintained libraries that must nearly always be included or resolving independent homespun implementations whenever a dev team grows past size 1.

Ultimately its the quality of functions in base, rather than its size, that Julia should be judged on. And well-written, standardized normalize functions should be a part of the linear algebra base.

Joseph

@johnmyleswhite
Copy link
Member

@josephnunn: I would suggest avoiding words like "ridiculous" when making arguments. It's seldom an effective rhetorical trope.

@Keno
Copy link
Member

Keno commented Oct 19, 2015

Unless there is an obvious package to put it in (and I don't think there is given that it is applicable to several domains), putting in in base seems fine.

@rofinn
Copy link
Contributor

rofinn commented Oct 19, 2015

@josephnunn I think a pretty good middle ground to address your concerns would be to place common functions like normalize into a stdlib or base math or stats pkg. This would address your concerns of having multiple "buggy or unoptimized" implementations kicking around while not adding further bloat to base. From what I understand this would also fit with the community's desire to break base out into a stdlib(s).

jiahao added a commit that referenced this issue Oct 20, 2015
Simple helper functions for normalizing vectors

Closes #12047
jiahao added a commit that referenced this issue Oct 20, 2015
Simple helper functions for normalizing vectors

Closes #12047
jiahao added a commit that referenced this issue Oct 20, 2015
Simple helper functions for normalizing vectors

Closes #12047
@josephnunn
Copy link

@johnmyleswhite Perhaps you are right on my choice of words, but as I mentioned for those used to working in certain problem areas incredulity will be the reaction, as it was for myself.

jiahao added a commit that referenced this issue Oct 21, 2015
Simple helper functions for normalizing vectors

Closes #12047
@c42f
Copy link
Member Author

c42f commented Oct 24, 2015

Great, thanks Jiahao. I started implementing this, but got hung up on the details. As your pull request shows, there are several, surprisingly.

bjarthur pushed a commit to bjarthur/julia that referenced this issue Oct 27, 2015
Simple helper functions for normalizing vectors

Closes JuliaLang#12047
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests