ACEsuit / ACE.jl

Parameterisation of Equivariant Properties of Particle Systems
66 stars 15 forks source link

The Constant (0-correlation) #86

Closed cortner closed 2 years ago

cortner commented 2 years ago

This PR finally brings the constant basis function backs. It's big fat pain, since it requires quite a few special cases, but it may be worth it long term. To deal with the fact that we sometimes want to remove it, I will implement a filter that does so. Good opportunity to test our filtering framework.

This will address #80.

CC @zhanglw0521

cortner commented 2 years ago

@zhanglw0521 is the basis for 0-correlations for the spherical Vector and spherical Matrix. It should depend on the Ls. E.g. for invariants the basis is just [ Invariant(1.0), ] .

For a euclidean vector, the basis is empty because there is no constant component (can't since it must rotate under rotation). I suspect for spherical the. basis is empty unless all Ls are zero? But maybe this is wrong for the spherical matrix, when L1+L2 = even?

zhanglw0521 commented 2 years ago

@zhanglw0521 is the basis for 0-correlations for the spherical Vector and spherical Matrix. It should depend on the Ls. E.g. for invariants the basis is just [ Invariant(1.0), ] .

For a euclidean vector, the basis is empty because there is no constant component (can't since it must rotate under rotation). I suspect for spherical the. basis is empty unless all Ls are zero? But maybe this is wrong for the spherical matrix, when L1+L2 = even?

I even think the constant term could only appear when L1 = L2 but I would think it through in case I spoke too soon.

cortner commented 2 years ago

please take your time to work this out properly

cortner commented 2 years ago

this seems ready now except for how to deal with he spherical tensors.

zhanglw0521 commented 2 years ago

Looks all good to me now. If I made no mistake, the constant terms can't exist for L1 != L2 as the initial coupling coefficients in this case are zeros. For those L1 == L2 cases, all possible non zero initial coupling coefficients are the same, hence, only one constant term appears. All these were done and checked by doing an SVD to initial coupling coefficients.

zhanglw0521 commented 2 years ago

Some necessary tests were added too.

cortner commented 2 years ago

thank you - will review asap

cortner commented 2 years ago

Liwei - if there is only one such basis function then it must necessarily be a multiple of the identity. Should that be hard-coded instead of the svd reduction that you do, and simply documented somewhere that this is the only constant basis function? Can it be easily proven?

cortner commented 2 years ago

Here is where I am nervous: if we took 2D, then rotations commute. So ALL rotation A matrices would satisfy Q * A * Q' = A for all Q. So it is definitely not trivial. Of course my argument doesn't work in 3D since rotations don't commute. Now of course we are in spherical coordinates, so the story changes again ... I'm inclined to believe the result, but can it be proven?

zhanglw0521 commented 2 years ago

Thanks for pointing out the 2d case. I'd still believe that 3d help us to get rid of nontrivial constant terms - will try to prove it (or disprove...).

cortner commented 2 years ago

ok, but maybe I will merge for now and we can make another PR later. At least the code right now looks sensible.

zhanglw0521 commented 2 years ago

Thanks, I'm happy with that - at least we could stick with the NoConstant case so that all functions work as usual.

cortner commented 2 years ago

ok, just waiting for tests to finish now then I'll squash and merge.