Fix and generalise the function `norm(X::XState)`

[zhanglw0521]

There seems to be a typo in norm(X::XState) associated with which the MWE looks like:

julia> Xs = PositionState([1.0,1.0,1.0])

julia> @show norm(Xs)
norm(Xs) = 2.9999999999999996

julia> @show norm(Xs.rr)
norm(Xs.rr) = 1.7320508075688772

and also it won't work for States that have non-number fields, e.g.,

julia> X = ACE.State(rr = [1.0,1.0,1.0], be = :env);
julia> norm(X)

ERROR: MethodError: no method matching iterate(::Symbol)

(A very similar issue occurs in scaling(basis::PIBasis) I guess?)

[cortner]

@zhanglw0521 --

(1) The first problem was a bug and should be fixed in #93. Can you check?

(2) The second problem is intended behaviour. I don't see how you can take the norm of this object. Analogy to elementary geometry might help. In geometry you have points and vectors. a point doesn't have a norm. but a vector is pointing from one point to another and it does have a norm - i.e. a well-defined length.

I'm happy with this behaviour, but if you disagree we can discuss further.

(3) I don't really see how scaling is at all related to this, but please file a separate issue if needed.

[zhanglw0521]

Thanks! I would consider all those things are fixed - the first one is indeed fixed with the latest version and as for the second one, I arose those comments simply because: (i) I wanted to add a test to OffsiteBasis, i.e., to check do I have a correct cutoff for it but of course I could use norm(X.rr) instead of norm(X) (Edit: I didn't use this simply because norm(X) \ne norm(X.rr) previously, but now it is corrected so that not a problem anymore); (ii) We didn't enable scaling() for a basis that has a component CatagoricalBasis at that stage but this is also fixed now. I really think we can close this issue, or may I close it now :)?

[cortner]

Yes, please close it. On Dec 27, 2021, 4:33 PM -0800, Liwei Zhang @.***>, wrote:

Thanks! I would consider all those things are fixed - the first one is indeed fixed with the latest version and as for the second one, I arose those comments simply because: (i) I wanted to add a test to OffsiteBasis, i.e., to check do I have a correct cutoff for it but of course I could use norm(X.rr) instead of norm(X); (ii) We didn't enable scaling() for a basis that has a component CatagoricalBasis at that stage but this is also fixed now. I really think we can close this issue, or may I close it now :)? — Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you commented.Message ID: @.***>