ACEsuit / ACE.jl

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

r0 has no effect if Polytransform used with multitransform #90

Closed WillBaldwin0 closed 2 years ago

WillBaldwin0 commented 2 years ago

I believe that the implementation of multitransform means that r0 has no effect if polytransform is used.

As far as I can tell, the current implementation of multitransform involves applying taking a given transform for a certain species, and composing it with an affine transformation to map each pair (r_in, r_cut) to (-1, 1).

This is done by taking the limits in x-space (letting x(r) denote the distance transform x --> r) which are x_cut = x(r_cut), x_in = x(r_in), and defining a new transform:

x_hat(r) = - 1 + (1 - - 1) / (x_cut - x_in) * (x(r) - x_cut)

This new transform maps (r_in, r_cut) to (-1, 1) with the same scaling effects that the original one had when mapping (r_in, r_cut) into (x_in, x_cut).

The implementation of the polytransform (with p=2) in ACE is

x(r, r0) = ((1+r0) / (1+r))^2

This has the following scaling symmetry, for constant a:

x(r, a.r0) -> c(a).((1+r0) / (1+r))^2 = c(a).x(r, r0)

This means that the affine transformation always undoes any change that we make by changing r0. I also made this interactive graph to demonstrate.

I have tested the affects of varying r0 with both the Agnesitransform and the PolyTransform, and I do indeed get identical fits when using the Polytransform, while I get a wide range of performance using the Agnesitransform with the same range of r0.

Changing the polytransform definition to

x(r) = (1/(r0+r))^p

might be a good solution? I think that I have actually seen the polytransform written in this way somewhere else, but I'm not sure where.

I don't think that this problem affects the use of polytransform out of multitransform, but I have not been able to understand the code that much yet.

cortner commented 2 years ago

thanks for sharing this observation.

My knee-jerk response is that I don't think this need be a problem at all. I introduced r0 primarily to make the transform non-dimensional and for numerical stability.

What you propose above is of course possible but it is a different sort of parameter that I would not call r0 anymore. Let me mull this over for a little bit?

cortner commented 2 years ago
x = (1 / (r0 + r))^p = ( (1/r0) / (1 + r/r0) )^p

Looking at this, I think I would prefer

x = (1 / (r0 + r))^p = ( a / (b + r/r0) )^p

That way we make it non-dimensional by design and still have plenty of parameters to encompass anything you need.

cortner commented 2 years ago

I'm going to close this since this is part of ACE1 which is no longer developed in the current repo. Please open a new issue there if this is still a concern and maybe link to this closed issue.