-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
add new vecnorm(x, p), generalizing and replacing normfro #6057
Conversation
Also, note that I changed the various |
@@ -400,6 +400,7 @@ eval(Sys, :(@deprecate shlib_list dllist)) | |||
IntSet(xs::Integer...) = (s=IntSet(); for a in xs; push!(s,a); end; s) | |||
Set{T<:Number}(xs::T...) = Set{T}(xs) | |||
|
|||
@deprecate normfro(A) vecnorm(A) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
normfro
should also be removed from exports.jl.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't it have to be in exports because it still exists, albeit in deprecated form?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, the @deprecate
macro automatically exports it for you (there's no point in having a private deprecation, after all).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, okay. Will fix.
Really nice. |
Can we get a NEWS.md snippet for the behavior change to |
end | ||
return a | ||
return float(minabs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not necessary to do the conversion here. It should be done in vecnorm
. This function is type stable without the conversion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just as easy to do the conversion here, and things are a bit more uniform if all the vecnormX
functions return the same type.
Actually, I considered the type instability of @pao I did that yesterday. |
It can definitely be nice to get the 1-norm or inf-form of an integer array as an integer, but I believe that the type instability might be a real performance issue. Maybe we should add |
I really don't see the practical utility of integer-valued norm functions. Vectors of integers don't form a Banach space... where would an integer-valued norm be useful? |
Full agree with this. Returning integer norms doesn't make any sense to me. |
Of all people, I think @stevengj is the last one we have to worry about not updating the docs. |
(I will add a NEWS item, but I usually add a NEWS item after the patch is merged.) |
Do vectors of floats form a Banach space? What has completeness to do with this? Are integers less real than floats? And what about the minus infinity norm? |
Vectors of floats are a reasonable approximation for vectors of reals, and are well-accepted as such. Vectors of integers are not. (Yes, integers are still subsets of the reals, but if you view them as a mere subset rather than as a sub-space then there is no need for the return type to be an Yes, we compute several things that are not norms. I was particularly skeptical of the utility of the |
The change to document isn't a part of this patch (it was already changed), so it seemed worth noting that as a separate item, particularly because it's a non-deprecable (deprecatable?) change. |
It is true that from a math standpoint that norms only make sense in Banach space. However, allowing vectors of integers as input would be quite handy especially in interactive sessions. For example, I think it would be reasonable to expect the following to work: vecnorm([1,2,3]) This may be interpreted as treating |
That's not the argument – the question is whether the 1-norm of an integer vector should be integral or floating point. And @stevengj and I are arguing that it doesn't make much sense to get an integral norm value even though in the case of the 1-norm the result happens to always be an integer. |
Ok. Then I agree with using floating point value for the norm. |
I actually do think that 1- and inf-norms make sense for integer matrices as well. Integer matrices are closed under matrix addition and multiplication, these vector norms give integer values on integer vectors, as do the induced norms on integer matrices, which in turn give exact bounds on the maximal amplification that an integer matrix will cause in the respective norms. Coincidence or not, there is clearly some mathematical structure here. That said, integer 1- and inf-norms are probably a niche enough application, and it's easy to roll your own. |
@toivoh, much of the concept of "linear algebra" goes out the window for integer matrices and vectors, because integer vectors are not a vector space (over the integers) because integers do not form a field (with the usual The question is, are there any real-world applications of integer-valued L1 norms in which returning a |
Oh, I definitely agree that norm and vecnorm should be type-stable. The |
The question of having different norm functions with various names was discussed back in #1875. |
In the context of wanting an integer answer for integer inputs, while what you're computing may happen to be the same as the 1-norm or the inf-norm, in such cases, I'd say you are doing abs-sum or abs-max reduction, rather than taking a norm. Exposing those computations as norms is weird. |
Yes, but that was before the type stability issue was brought up. Anyway, |
@stevengj If you insist that the input type represents the field of the vector space then think of the Julia type hate most: However, my math arguments won't make the type inference problem go away. |
@andreasnoackjensen, I don't want to restrict the input type, I want to make the output type floating-point. We should certainly accept rational (and integer) vectors as input, but it is insane for the L1 norm (for instance) to try to return a rational output (= near-certain overflow for fixed-precision types, and exponential complexity for Even when we can practically compute a rational or integer result from a (PS. While vectors over the rationals are indeed a vector space, the are not a Banach space because they are not complete.) |
Be it resolved that norm returns an appropriate floating-point type. |
So is the patch okay to merge? |
add new vecnorm(x, p), generalizing and replacing normfro
I think it is unfortunate to promote when there is no reason to do it. For A cool thing about Julia is that you can write very generic code such that you can define a new It would have costed you ten seconds of work to acknowledge that point and move the promotion to floating point from |
You should never call them elsewhere because they don't even handle the I don't see the point in duplicating code, even if it is a few simple calls, and risking overflows to return a result no one wants from functions no one will call. |
At the risk of jumping in without reading the whole thread --- I still hate the name i'm okay with both existing, and no depracating my two cents |
As discussed on the mailing list, this adds a new
vecnorm(x,p)
function (generalizing and replacingnormfro
) which computes a p-norm of any iterable container as if it were a vector.Along the way, I also made a few other improvements, e.g. the generic-
p
case no longer allocates temporary arrays, accumulation is done in double precision forFloat16
orFloat32
arrays (but the result is still returned asFloat16
orFloat32
), andcheapnorm
inquadgk
is eliminated in favor ofvecnorm
.Also fixes #6056 (makes
norm
type-stable).cc: @toivoh, @StefanKarpinski